Rock, Paper, Scissors in JavaScript

Question

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);

Answer 1

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";
}

Answer 2

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.

Answer 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.

Answer 4

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));

Answer 5

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";
            }
        }
    }
}