15 replies to this topic

[Adderly]

So basically, I'm trying to write a pretty basic Rock, Paper, Scissors console game. Here's what I have so far:

public class RockPaperScissors {
	public static void main(String[] args) {
		String win = "You win!";
		String lose = "Sorry. You lose.";
		String computerMove;
		String selection;
		int myMove;
		int rand;
		
		
		//Get myMove
		
		TextIO.putln("What's your move?");
		TextIO.putln();
		TextIO.putln("	1. Rock");
		TextIO.putln("	2. Paper");
		TextIO.putln("	3. Scissors");
		myMove = TextIO.getlnInt();
		
		//Get computerMove
		
		rand = (int)(3*Math.random());
		if ( rand == 0)
			computerMove = "Rock";
		else if (rand == 1)
			computerMove = "Paper";
		else
			computerMove = "Scissors";
		
		//Compare moves
		
		switch (myMove) {
		case 1:
			selection = "Rock";
				if (computerMove == "Paper")
					TextIO.putln(lose);
					TextIO.putln(computerMove);
					TextIO.putln(" beats ");
					TextIO.putln(selection);
				else
					TextIO.putln(win);
					TextIO.putln(selection);
					TextIO.putln(" beats ");
					TextIO.putln(computerMove);
				}
					
		}
	}

The compiler is saying the "ELSE" statement in the case labeled "Case 1" is giving me the error: "Syntax error on token "else", delete this token". What can I do to remedy this? Thanks in advance.

PS. I posted a topic earlier, but it's subject has nothing to do with the content of this one. If I've created too many topics, please feel free to let me know so I can move this into the prior topic. Thanks again.

[Palidine]

Just add parens to the case block to scope things properly

switch
{
    case 0:
    {
        if (blah blah)
        {
        }
        else
        {
        }

    }
}

[owl]

don't you need to use brackets when the if has multiple lines?

[Hodgman]

Your if and your else inside switch (the ones it's complaining about) don't have any braces ({ }) on them!
If an if/else has more than one statement inside it, those statements need to be contained within braces.

This is what Java is seeing:

if (computerMove == "Paper")
            TextIO.putln(lose);
        //the 'if' just ended here
        TextIO.putln(computerMove);//this is outside of the if
        TextIO.putln(" beats ");
        TextIO.putln(selection);
        else //wtf, why is there an 'else' here that isn't connected to an 'if'?? ZOMG I'll report an error telling the user to delete it.
            TextIO.putln(win);
        TextIO.putln(selection);
        TextIO.putln(" beats ");
        TextIO.putln(computerMove);

[Adderly]

I was just about to edit my initial post to ask that. But wouldn't it alter what the computer sees in and outside the brackets?

For example:

computerMove is initialized

{computerMove isn't initialized}

Thanks again!

[Adderly]

Your if and your else inside switch (the ones it's complaining about) don't have any braces ({ }) on them!
If an if/else has more than one statement inside it, those statements need to be contained within braces.

This is what Java is seeing:

if (computerMove == "Paper")
            TextIO.putln(lose);
        //the 'if' just ended here
        TextIO.putln(computerMove);//this is outside of the if
        TextIO.putln(" beats ");
        TextIO.putln(selection);
        else //wtf, why is there an 'else' here that isn't connected to an 'if'?? ZOMG I'll report an error telling the user to delete it.
            TextIO.putln(win);
        TextIO.putln(selection);
        TextIO.putln(" beats ");
        TextIO.putln(computerMove);

Thank you for explaining in detail! Detail is always helpful I was aware that I could specifically define that multiple lines of code are within an 'if' statement with brackets, but I was afraid it would blind the compiler to everything outside of it.

[Adderly]

So here's the final product (with a few changes from my initial post):

public class RockPaperScissors {
	public static void main(String[] args) {
		String win = "You win!";
		String lose = "Sorry. You lose.";
		String computerMove;
		String selection;
		int myMove;
		int rand;
		
		
		//Get myMove
		
		TextIO.putln("What's your move?");
		TextIO.putln();
		TextIO.putln("	1. Rock");
		TextIO.putln("	2. Paper");
		TextIO.putln("	3. Scissors");
		myMove = TextIO.getlnInt();
		
		//Get computerMove
		
		rand = (int)(3*Math.random());
		if ( rand == 0)
			computerMove = "Rock";
		else if (rand == 1)
			computerMove = "Paper";
		else
			computerMove = "Scissors";
		
		//Compare moves
		
		switch (myMove) {
		case 1:
			selection = "Rock";
				if (computerMove == "Paper")
					{TextIO.putln(lose);
					TextIO.putln();
					TextIO.putln(computerMove + " beats " + selection);}
				else if (computerMove == "Rock")
					TextIO.putln("Tie!");
				else
					{TextIO.putln(win);
					TextIO.putln();
					TextIO.putln(selection + " beats " + computerMove);}
				break;
		case 2:
			selection = "Paper";
				if (computerMove == "Scissors")
				{TextIO.putln(lose);
				TextIO.putln();
				TextIO.putln(computerMove + " beats " + selection );}
				else if (computerMove == "Paper")
					TextIO.putln("Tie!");
				else
				{TextIO.putln(win);
				TextIO.putln();
				TextIO.putln(selection + " beats " + computerMove);}
			break;
		case 3:
			selection = "Scissors";
				if (computerMove == "Rock")
				{TextIO.putln(lose);
				TextIO.putln();
				TextIO.putln(computerMove + " beats " + selection);}
				else if (computerMove == "Scissors")
					TextIO.putln("Tie!");
				else
				{TextIO.putln(win);
				TextIO.putln();
				TextIO.putln(selection + " beats " + computerMove);}
			break;
		default:
			TextIO.putln("You can't choose that! I quit!");
			System.exit(1);
				}
					
		}
	}

Is there anything anyone would've done differently? Also, rather than breaking from the case statement and ending the program, how can I go back to the beginning of the main class to start the game again?




edit: Nevermind. I'm pretty sure I can implement a boolean expression to determine whether or not the user would like to play again. (Although I'm not sure how to return to the beginning of the main class). Still, though. What would you have done differently?

[Danny02]

challenge accepted :-)

public static void main(String[] args) {
        String[] choices = new String[]{"Rock", "Paper", "Scissors"};
        System.out.println("What's your move?");
        for (int i = 0; i < 3; ++i)
            System.out.println("\t" + (1 + i) + ". " + choices[i]);
        Scanner in = new Scanner(System.in);
        int human = in.nextInt() - 1; //Get myMove
        int pc = (int) (3. * Math.random());//Get computerMove

        System.out.println("Computer chooses: " + choices[pc]);
        System.out.println("You choosed: " + choices[human]);

        int i = human - pc;
        if (pc == human)
            System.out.println("Tie!");
        else if (i == 1 || i == -2)
            System.out.println("You win!");
        else
            System.out.println("Computer wins!");
    }

for such a little programm a while loop will do, break out if the player don'T want to play another game.


ps: i nearly forgot, never ever compare two strings with ==. Always use the equals method or isEmpty.
a.equals("b") instead of a == "b"
a.isEmpty() instead of a == ""

[Adderly]

challenge accepted :-)

public static void main(String[] args) {
        String[] choices = new String[]{"Rock", "Paper", "Scissors"};
        System.out.println("What's your move?");
        for (int i = 0; i < 3; ++i)
            System.out.println("\t" + (1 + i) + ". " + choices[i]);
        Scanner in = new Scanner(System.in);
        int human = in.nextInt() - 1; //Get myMove
        int pc = (int) (3. * Math.random());//Get computerMove

        System.out.println("Computer chooses: " + choices[pc]);
        System.out.println("You choosed: " + choices[human]);

        int i = human - pc;
        if (pc == human)
            System.out.println("Tie!");
        else if (i == 1 || i == -2)
            System.out.println("You win!");
        else
            System.out.println("Computer wins!");
    }

for such a little programm a while loop will do, break out if the player don'T want to play another game.

lol! Show off




As a beginner, pieces of your solution looks foreign to me. Would you care to elaborate more with comments to specify what you're doing and when? If not, you're not obligated to. I'm just in Learning Mode, today because I'm happy that I achieved the results that I wanted to. This is the first interactive program I've started from scratch. And no matter how newbish my solution may be, I'm proud of it!




edit: By the way, you forgot to include 'import java.util.Scanner;'

[Danny02]

Just ask about the parts u don't understand.

I think the top part of the code should be quite clear.

Instead of "TextIO" I use the standard utils to output and get values from the standart Streams(Console).

System.out.println("What's your move?");
...Scanner in = new Scanner(System.in);
int human = in.nextInt() - 1

After that the player and the pc has chosen a value in the range [0,2]. Where 0:Rock, 1:Paper and 2:Sicssors.
Now when u look at the numbers and compare them with the game rules u can see, that a number always wins against the next smaller one(1 vs 0, 2 vs 1).
There are 3 different combinations and because of that we need only one more rule to decide who is the winner(have already 2 above).

We check first of course if both choices are the same and call it a "Tie".
after that we subtract both choices from each other and will get -1, 1, -2 or 2 depending against which player we want to check we check for (1, -2) or (-1, 2)

[rip-off]

Danny02's code is clever in that it works almost entirely on the integer used to represent a move. The move names are stored in a table that can be quickly consulted by the move's numeric code. The winner is determined by some tricksy manipulation of the numeric representation.

My main criticism of such an approach is that it invests a lot into the "magic numbers" being used. For a known game like Rock, Paper, Scissor this is fine, but if you were writing a bigger game this approach is brittle, any changes to the number of moves can result in the code breaking in subtle ways without any compilation errors.

I would also comment on your code OP, you are comparing objects using "==". This is not a good idea as this compares the objects "identity" rather than their value. Strings are objects, you should use equals() method, like: computerMove.equals("Rock")

I think the program is best written using an enumeration representing all available moves:

public class RockPaperScissor {

    public enum Move {
        Rock, Paper, Scissor
    }
    
    private static final Move [] Moves = Move.values();
    
    public static Move getPlayerMove() {
        int index;
        do {
            TextIO.putln("What's your move?");
            TextIO.putln();
            for(int i = 0 ; i < Moves.length ; ++i) {
                TextIO.putln(i + " " + Moves[i]);
            }
            index = TextIO.getlnInt();
        }
        while(index < 0 || index > Moves.length);
        
        return Moves[index];
    }
    
    private static Move getComputerMove() {
        int index = (int)(Moves.length * Math.random());
        return Moves[index];
    }

    private static boolean beats(Move player, Move computer) {
        switch(player) {
        case Rock:    return computer == Move.Scissor;
        case Paper:   return computer == Move.Rock;
        case Scissor: return computer == Move.Paper;            
        }
        
        // Hush little compiler, don't you cry
        throw new IllegalArgumentException("Unknown player move: " + player);
    }

    public static void main(String[] args) {
        Move player = getPlayerMove();
        Move computer = getComputerMove();
        
        TextIO.putln("Computer chose: " + computer.toString());
        if(player == computer) {
            TextIO.putln("Tie!");
        } else {
            boolean win = beats(player, computer);
            if(win) {
                TextIO.putln(player + " beats " + computer);
                TextIO.putln("You win!");
            } else {
                TextIO.putln(computer + " beats " + player);
                TextIO.putln("Sorry. You lose.");
            }
        }
    }         
}

If you wanted to add an additional moves, to make rock-paper-scissor-bazooka or whatever, there would be a lot less work to do. Also, I believe the code is much clearer than the clever but hard to decipher solution presented earlier. I also like when a program is split up into lots of small, east to understand functions.

[Adderly]

Excellent, rip-off! I can understand most of your code, but there are a still few parts that look new to me. The book that I'm reading now is just getting into exceptions and 'try..catch'. Haven't seen anything on 'throw new' yet, but I'm sure I will. Also, thank you for informing me of the difference between the "==" operator and 'equals()' method. I'll certainly consider that for future application. Like I said earlier though, this was the first program I'd written from scratch to suit my desires and I thought it turned out wonderfully, considering I've only been really reading heavily for about a week now. I'm still proud of it. I've finally stepped over that boundary of writing a functional (however arbitrary it may be) and interactive program. For the first time since I started reading, I feel like I might actually have a shot at this once I have some experience behind me.




Thank you both for you contributions! Everyone here has helped immensely and I greatly appreciate that. I'm glad I've joined the community.

[rip-off]

For a first attempt your original code is quite good. The main weakness was the aforementioned equality bug. Other, minor points to consider are delaying variable declarations to where they are first initialised/used, and reducing the repetition of code for handling win/loss inside each "case" block.

Another thing is to have a clean, consistent code style. In particular the brace placement and indentation of your switch statement is inconsistent.

There are two common styles:

control statement {
    // Statments indented one "logical unit" here
}

// and 

control statement
{
    // Statments indented one "logical unit" here
}

Pick one you like the look of and go with it.

Some code styles allow for single line control blocks to omit the braces. However, most people believe this is a poor idea as you can all too easily add a second line without remembering to put the braces in. I would recommend adopting a style that always using braces around any control block.

[Adderly]

Thank you! That actually means a lot. I'll certainly keep your comments in mind. I didn't even think of holding off on declaring variables until it came time to use them. When I first chalked it up in pseudocode, I wrote through the entire function to itemize the variables and types I would need and carelessly declared them at the beginning of the code.

Yeah, I noticed the inconsistency yesterday upon completion and went back to fix it. I come from a background in HTML, CSS, and some Javascript and a little experience in PHP, so I'm not sure why I didn't do it from the beginning. Haste, I suppose.

I plan on returning to this particular project in the future to compare how I do with a bit more familiarity with Java.

[Danny02]

some thoughts, I thought about using an enum but for this small program there weren't really a benefit for it(also I tried to write it as small as possible :-) ). Also I didn't wanted to confuse Adderly cause enums are a bit complicated to understand especially in Java.


Of course if u want to extend the choices I probably have done it with an enum, extending it even so that each enum value has an EnumSet against which enum it wins.

On thing more, if I wanted to to it right. I would probably use States for the game(decide pick, want t play again?, enter name for highscore etc.).

@rip-off why do u throw an exception^^
I know, the throw is unreachable but it just don't look right. I think in Java it is also a good habit to only have 1 return in a method(the compiler can optimize better, but who cares :-) )

boolean wins = false;

switch(player) { 
   	case Rock:    wins = computer == Move.Scissor; 
   	case Paper:   wins =  computer == Move.Rock; 
   	case Scissor: wins =  computer == Move.Paper; 
}
return wins;

[rip-off]

some thoughts, I thought about using an enum but for this small program there weren't really a benefit for it(also I tried to write it as small as possible :-) ). Also I didn't wanted to confuse Adderly cause enums are a bit complicated to understand especially in Java.

I don't think enums are particularly confusing. They have a few unusual properties, but they are mostly intuitive. I think they are less confusing than magic numbers.

why do u throw an exception^^

No particular reason, but I don't think it makes sense to continue processing in the case where the Move is not catered for. For example, if such a function was used by AI logic it could end up in an infinite loop (i.e. a general "is move A better than move B").

I think in Java it is also a good habit to only have 1 return in a method

boolean wins = false;

switch(player) { 
        case Rock:    wins = computer == Move.Scissor; 
        case Paper:   wins =  computer == Move.Rock; 
        case Scissor: wins =  computer == Move.Paper; 
}
return wins;

I think this is limiting. I think early returns are a great way of getting edge cases out of the way quickly. Your code now contains a bug: there are no break statements between the cases, resulting in fall though.

(the compiler can optimize better, but who cares :-) )

I don't believe this is the case. Do you have any evidence for this assertion?