9

I'm working on making my first game (Rock Paper Sissors) and I ran into an issue where when the userChoice is scissors and the computerChoice is rock, the program cannot return the winner as rock. I can get the program to give me the winner for any other combination.

I have my code here:

var userChoice = prompt("Do you choose rock, paper or scissors?");
var computerChoice = Math.random();
if (computerChoice < 0.34) {
    computerChoice = "rock";
} else if(computerChoice <= 0.67) {
    computerChoice = "paper";
} else {
    computerChoice = "scissors";
}

var compare = function(choice1, choice2) {
    if(choice1 === choice2) {
    return "The result is a tie!";
}
if(choice1 === "rock") {
    if(choice2 === "scissors") {
        return "rock wins";
    } else {
        return "paper wins";
    }
}
if(choice1 === "paper") {
    if(choice2 === "rock") {
        return "paper wins";
    } else {
        if(choice2 === "scissors") {
            return "scissors wins";
    }
}
if(choice1 === "scissors") {
    if(choice2 === "rock") {
        return "rock wins";
    } else {
        if(choice2 === "paper") {
            return "scissors wins";
        }
    }
}
}
};
console.log("User Choice: " + userChoice);
console.log("Computer Choice: " + computerChoice);
compare(userChoice, computerChoice);
4
  • There was a syntax error missing required brace or something at the end of the function, and you inserted it there? Don't believe them, the brace was missing somewhere else but the parser did recognice that at the end.
    – Bergi
    Jul 31, 2013 at 17:26
  • 2
    This code, once fixed, should be sent to Code Review, it can be optimized by a lot.
    – Jeff Noel
    Jul 31, 2013 at 17:28
  • 1
    they solved your problem. but you should be using "else if" when its appropriate like in your code.. also do you really need all those {} ? cleaner code has less mistakes
    – gezzuzz
    Jul 31, 2013 at 17:31
  • @gezzuzz, I'm not sure if I need all of the {}... I just started learning JavaScript two days ago at Codecademy. Could you show me a working example without all of the braces?
    – Godzdude
    Jul 31, 2013 at 17:51

14 Answers 14

18

Something to study:

var choices = ["rock", "paper", "scissors"];
var map = {};

choices.forEach(function(choice, i) {
    map[choice] = {};
    map[choice][choice] = "Was a tie"
    map[choice][choices[(i+1)%3]] = choices[(i+1)%3] + " wins"
    map[choice][choices[(i+2)%3]] = choice + " wins"
})

function compare(choice1, choice2) {
    return (map[choice1] || {})[choice2] || "Invalid choice";
}

Here's an alternate that will work for expanded sets. The assumption is that there's an odd number of possibilities, and from any given point, of the total number of opposition, reading forward from our given point (and wrapping around when we reach the end) the first half will win over the given point, and the second half will lose.

Or another way to describe it would be that the half of the remaining opponents that precede our given point will lose, and the half that follow will win.

Therefore the proper order in the choices Array is crucial.

var choices = ["rock", "spock", "paper", "lizard", "scissors"];
var map = {};

choices.forEach(function(choice, i) {
    map[choice] = {};
    for (var j = 0, half = (choices.length-1)/2; j < choices.length; j++) {
        var opposition = (i+j)%choices.length
        if (!j)
            map[choice][choice] = "Was a tie"
        else if (j <= half)
            map[choice][choices[opposition]] = choices[opposition] + " wins"
        else
            map[choice][choices[opposition]] = choice + " wins"
    }
})

function compare(choice1, choice2) {
    return (map[choice1] || {})[choice2] || "Invalid choice";
}
3
  • @gezzuzz: I agree. That's why it's something to study. There are several concepts at play, each of which are very simple, but all together are perhaps a little mind-blowing.
    – user2437417
    Jul 31, 2013 at 19:52
  • I was having a hard time trying to understand this solution, but it inspired me to come up with a different one that I think is much simpler. Also, here you have to generate all possible comparisons, right? Jul 31, 2013 at 23:25
  • @PauloAlmeida: Yeah, for each option, it's just pre-generating a map of results when compared to to its opposing options. Half of the opposing options will win, the other half will lose.
    – user2437417
    Aug 1, 2013 at 14:27
10

You were unable to see the issue most likely due to poor indentation of your code. Properly indented the issue is clear:

if (choice1 === "paper") {
    if (choice2 === "rock") {
        return "paper wins";
    } else {
        if (choice2 === "scissors") {
            return "scissors wins";
        }
    }
    if (choice1 === "scissors") {
        if (choice2 === "rock") {
            return "rock wins";
        } else {
            if (choice2 === "paper") {
                return "scissors wins";
            }
        }
    }
}

Your if (choice1 === "scissors") { is within if (choice1 === "paper") {. The code within will never be reached.

4
  • Okay, So I'm just missing one curly brace then. Thanks!
    – Godzdude
    Jul 31, 2013 at 17:31
  • I'm pretty sure you have the right number of braces. I guess you could say they're in the wrong order. You need to move the code outside of the other if. Jul 31, 2013 at 17:33
  • Well, wouldn't putting one more brace above if(choice1 === "scissors") effectively move it outside of the other if statement?
    – Godzdude
    Jul 31, 2013 at 17:47
  • Only if you also remove one of the braces at the end. Otherwise you will have more closing braces than opening. Jul 31, 2013 at 19:24
3

I came up with an alternative that should be easy for you to understand and avoids some issues in your code, like excessive repetition and fixed choices. It is thus much more flexible and easier to maintain.

function compare(choice1, choice2) {
    choice1 = choices.indexOf(choice1);
    choice2 = choices.indexOf(choice2);
    if (choice1 == choice2) {
        return "Tie";
    }
    if (choice1 == choices.length - 1 && choice2 == 0) {
        return "Right wins";
    }
    if (choice2 == choices.length - 1 && choice1 == 0) {
        return "Left wins";
    }
    if (choice1 > choice2) {
        return "Left wins";
    } else {
        return "Right wins";
    }
}

choices is var choices = ["rock", "paper", "scissors"];. You can see a demonstration.


To generalize the solution to larger lists, this modulo technique can be helpful:

function mod(a, b) {
    c = a % b
    return (c < 0) ? c + b : c
}

Then it's much easier to write the comparison code:

function compare(choice1, choice2) {
    x = choices.indexOf(choice1);
    y = choices.indexOf(choice2);
    if (x == y) {
        return "Tie";
    }
    if (mod((x - y), choices.length) < choices.length / 2) {
        return choice1 + " wins";
    } else {
        return choice2 + " wins";
    }
}

The corresponding jsFiddle.

0
2

So many if statements. They are confusing.

Also, all those if statements lock in the game and make it hard to reuse the logic for another game.

function referee(){
    var training = {};
    function learn(winner,loser){
        if (!training[winner]) training[winner] = {};
        training[winner][loser]=1;
    }
    function judge(play1,play2){
        if (play1 === play2){ return 'tie'; }
        return ( (training[play1][play2] === 1)? play1: play2 )+' wins!';
    }
    function validate(choice) {
        return choice in training;
    }
    function choices() {
        return Object.keys(training);
    }
    return {
        'learn': learn,
        'judge': judge,
        'validAction': validate,
        'getChoices': choices
    };
}

var ref = referee();
ref.learn('rock','scissors');
ref.learn('paper','rock');
ref.learn('scissors','paper');

do {
    var userChoice = prompt("Do you choose rock, paper or scissors?");
} while(!ref.validAction(userChoice))
var choices = ref.getChoices(),
    computerChoice = choices[Math.floor(Math.random()*choices.length)];

console.log("User Choice: " + userChoice);
console.log("Computer Choice: " + computerChoice);
console.log(ref.judge(userChoice, computerChoice));
7
  • 2
    The reason I'm using if statements is because I just started learning JavaScript 2 days ago at Codecadamy. I haven't learned most of the things you've used to create the cleaner coded version of the game.
    – Godzdude
    Jul 31, 2013 at 17:56
  • 1
    That's ok. We all start somewhere. Sometimes you only learn these tricks by the end of a long course, but I think it could be good to learn them sooner.
    – Paul
    Jul 31, 2013 at 18:01
  • Tested and expaneded a bit now :-)
    – Bergi
    Jul 31, 2013 at 18:02
  • Yes, it get's hoisted.
    – Bergi
    Jul 31, 2013 at 18:06
  • 2
    And +1 of course, I love how this could be adapted to rock-paper-scissors-lizard-spock :-)
    – Bergi
    Jul 31, 2013 at 18:07
1

You have a mismatched brace:

if(choice1 === "paper") {
    if(choice2 === "rock") {
        return "paper wins";
    } else {
        if(choice2 === "scissors") {
            return "scissors wins";
    }
}

I'd actually remove the last if in that block, you don't need it. The last block (choice1 === "scissors") is correct, but again the last if is not required.

To show you why it's failing in that particular way, I have re-indented the relevant part of your code to illustrate how it is being interpreted:

if(choice1 === "paper") {
    if(choice2 === "rock") {
        return "paper wins";
    } else {
        if(choice2 === "scissors") {
            return "scissors wins";
        }
    }
    if(choice1 === "scissors") {
        if(choice2 === "rock") {
            return "rock wins";
        } else {
            if(choice2 === "paper") {
                return "scissors wins";
            }
        }
    }
}
1

Try This :

var UserChoice = window.prompt("Do you choose rock, paper or scissors ?");

var computChoice = Math.random();

var computChoice = computChoice < 0.34 ? "rock" : ( computChoice > 0.67 ? "scissors" : "paper" ) ;

var mess = { 
  rock : { scissors : 'You Win!, Rock smashes scissors!', paper : 'You lose!, Paper covers rock!'} ,
  paper : { rock : 'You Win!, Paper covers rock!', scissors : 'You lose!, Scissors cut paper!' },
  scissors : { paper : 'You Win!, Scissors cut paper!', rock : 'You lose!, Rock smashes scissors!' }
}

if ( computChoice === UserChoice)
  result = "It's a tie!" ; 
	 
else if ( UserChoice !== "rock" && UserChoice !== "paper" && UserChoice !== "scissors" )
  result = "Invalid choice! Choose from rock, paper, or scissors" ;

else
  result = mess[UserChoice][computChoice] ;

console.log( 'you chose ' + UserChoice + ' and computer chose ' + computChoice + ' ( ' + result + ' ) ') ;

1

This is how I did mine:


    //player choice
    var playerChoice = prompt("What is your choice of weapon: rock, paper, or scissors?");
    
    //Computer Choice
    var computerChoice = Math.ceil(Math.random() *3);
    
    //variables as numbers
    if (computerChoice < 1) {
        computerChoice = "rock";
    } else if(1 <= computerChoice <= 2) {
        computerChoice = "paper";
    } else {
        computerChoice = "scissors";
    }
    
    
    //defining function
    function game(playerChoice, computerChoice){
    
    //Checking for a tie
    if (playerChoice === computerChoice) {
          return "It is a tie";
        }
    
        //Check for Rock
        if (playerChoice === "rock") {
          if (computerChoice === "scissors") {
            return "Player Wins";
          } else {
            return "Computer Wins";
          }
        }
        //Check for Paper
        if (playerChoice === "paper") {
          if (computerChoice === "scissors") {
            return "Computer Wins";
          } else {
            return "Player Wins";
          }
        }
        //Check for Scissors
        if (playerChoice === "scissors") {
          if (computerChoice === "rock") {
            return "Computer Wins";
          } else {
                    return "Player Wins";
          }
        }
    }
    
    //start the game function
    game();
    //print winner
    console.log(game(playerChoice, computerChoice))

I chose to have my computerChoice be random, whole number, 0-3, but you can take that part out.

0

I got this to work:

function playFunction() {
var userChoice = prompt("Do you choose rock, paper or scissors?");
var computerChoice = Math.random();
if (computerChoice < 0.34) {
    computerChoice = "rock";
} else if(computerChoice <= 0.67) {
    computerChoice = "paper";
} else {
    computerChoice = "scissors";
}

var compare = function(choice1, choice2) {
    if(choice1 === choice2) {
      alert("The result is a tie!");
}
if(choice1 === "rock") {
    if(choice2 === "scissors") {
        alert("rock wins");
    } else {
        alert("paper wins");
    }
}
if(choice1 === "paper") {
    if(choice2 === "rock") {
        alert("paper wins");
    } else {
        if(choice2 === "scissors") {
            alert("scissors wins");
    }
}
if(choice1 === "scissors") {
    if(choice2 === "rock") {
        alert("rock wins");
    } else {
        if(choice2 === "paper") {
           alert("scissors wins");
        }
    }
}
}
};
console.log("User Choice: " + userChoice);
console.log("Computer Choice: " + computerChoice);
compare(userChoice, computerChoice)
} 

All I changed was instead of returning a message, it pops up an alert with the answer. I also put it into one function that could be called on an HTML button click.

0

Example without all the {} and else if

always use else if when you can.. since your if statements are different cases and only one applies you should use else if..

with if statements if you only have 1 statement after the condition you don't need {} (condition 1 below).. even if you have a if.. else if...block statement its considered one statement..(condition 2 below).. but if it helps you can use them around the if.. else if...block statement to help your understand it better ..(condition 3 below).. ..

also don't use === unless you really know what it does.. it can cause you trouble being a rookie.. use == by default..

if(choice1 == choice2)  //condition 1
    return "The result is a tie!";
else if(choice1 == "rock") //condition 2
    if(choice2 == "scissors") 
        return "rock wins";
     else 
        return "paper wins";
else if(choice1 == "paper"){ //condition 3
    if(choice2 == "rock") 
        return "paper wins";
     else 
        return "scissors wins";
}
else if(choice1 == "scissors")
    if(choice2 == "rock")
       return "rock wins";
    else 
       return "scissors wins";
2
  • I think getting rid of the curly braces is a good way to clean it up, though you left one pair for some reason. But I must say I completely disagree with your advice to use == over ===. That's typically the opposite of what people would advise. The === is a very simple equality comparison, while the == uses a much more complex algorithm when disparate types are compared in order to coerce the operands to matching types. It's that coercion algorithm that catches people off guard.
    – user2437417
    Jul 31, 2013 at 19:58
  • i left it for condition 3 to show him that you can still use it optionally to help you read..
    – gezzuzz
    Jul 31, 2013 at 20:40
0
var userChoice = prompt("Do you choose rock, paper or scissors? ");


var computerChoice=Math.random();

{


if(computerChoice <= ".33") 

{
    computerChoice === 'rock';


    }


    else if(computerChoice<='.66' & '>=.34')


    {

computerChoice === 'paper';


        }

        else

{

            computerChoice ===' scissors';


            }


            }


console.log( computerChoice);
0
0
var compare = function (choice1, choice2)
{
    if (choice1 === choice2)
    {
        return "The result is a tie!";
    }
    else
    {
        if(choice1 === "rock")
        {
            if(choice2 === "paper")
            {
               return "Paper beats rock. Computer Wins.";
            }
            else
            {
                return "Rock beats scissors. You win.";

            }
        }
        else
        {
            if(choice1 === "paper")
                {
                     if(choice2 === "rock")
                        {
                             return "Paper beats rock. You Win.";
                        }
            else
                {
                return "Scissors beat paper. Computer Wins.";               }

                }
    if(choice1 === "scissors")
                {
                     if(choice2 === "rock")
                        {
                             return "Rock beats scissors. Computer Wins.";
                        }
            else
                {
                return "Scissors beat paper. You Win.";               }

                }
        }
    }



};
var r = function(user)
{
    while(user < 0 | user >3)
    {user = prompt("Please don't act oversmart. Press '1' for rock, '2' for paper, and '3' for scissors.");
    }

    if(user === "1")
    user = "rock";

    else
    {
        if(user === "2")
        {user = "paper";}
        else
        {user = "scissors";}
    };
    console.log("You chose: " + user);

    computerChoice = Math.random()
    if(computerChoice <= 0.33)
    {
        computerChoice = "rock";
    }
    else
    {
        if(computerChoice > 0.33 && computerChoice <=0.66)
        {computerChoice = "paper";}
        else
        {computerChoice = "scissors";}
    }

    console.log("The computer chose: "+computerChoice)
    console.log(compare(user, computerChoice));
    if(user===computerChoice)
    {
        userChoice = user;
        return "1";}

};


var userChoice = prompt("Press '1' for rock, '2' for paper, and '3' for scissors")
var computerChoice;

var a = r(userChoice);
if(a === "1")
{//console.log("1");
while(userChoice === computerChoice)
{
    var a = prompt("Since there was a tie, please choose again. Press 1 for rock, 2 for paper and 3 for scissors.")
    var b = r(a);
    if(b !== "1")
    {break;}
}
}
1
  • 2
    You should really explain what is going on here, rather than just dumping this code.
    – Léo Natan
    Aug 16, 2014 at 14:13
0

This one will create the perfect, self repeating game until someone has won. It also shows you how many games you played. All without using loops!

count = 1;

var Decisions = function() {
    if (count === 1) {
        userChoice = prompt("Do you choose rock, paper or scissors?");
    } else {
        userChoice = prompt("It's a tie. Please make your choice again!");
    }
    computerChoice = Math.random();

    if (computerChoice < 0.4) {
        computerChoice = "rock";
    } else if(computerChoice <= 0.8) {
        computerChoice = "paper";
    } else {
        computerChoice = "scissors";
    }
    console.log("User: " + userChoice);
    console.log("Computer: " + computerChoice);
}

Decisions();

var compare = function(choice1, choice2) {
    if (choice1 === choice2) {

        count = count + 1
        console.log("The result is a tie!");
        Decisions();
        return compare(userChoice, computerChoice);

    } else if (choice1 === "rock") {
        if (choice2 === "scissors") {
            return "rock wins";
        } else {
            return "paper wins";
        }
    } else if (choice1 === "paper") {
        if (choice2 === "rock") {
            return "paper wins";
        } else {
            return "scissors wins";
        }
    } else if (choice1 === "scissors") {
        if (choice2 === "paper") {
            return "scissors win";
        } else {
            return "rock wins";
        }
    }
}

console.log(compare(userChoice,computerChoice));
console.log("Wow, you played " + count + " times!");
0

This is the code I made at this exercise and it worked like a charm... I used logical operators on my "if" statements, and it was accepted(obviously).

Give it a try :D

var userChoice = prompt("Do you choose rock, paper or scissor?");
var computerChoice = Math.random();
if (computerChoice > 0 && computerChoice < 0.33) {
  computerChoice = "Rock";
} else if (computerChoice > 0.34 && computerChoice < 0.67) {
  computerChoice = "Paper";
} else {
  computerChoice = "Scissor";
}
console.log(computerChoice);

0

The solution I came to as a fellow novice seems relatively simple..

var userChoice = prompt ("Do you choose rock, paper or scissors?");

var computerChoice = Math.random();
console.log(computerChoice);

if (computerChoice <=0.33) {
    "rock";
} else if (computerChoice <=0.66) {
    "paper";
} else {
    "scissors";
}
1
  • This doesn't answer the question.
    – VFDan
    Aug 7, 2021 at 21:49

Not the answer you're looking for? Browse other questions tagged or ask your own question.