HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Is there a way to make my Rock, Paper, Scissors game more concise?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
paperscissorsmakewaymoreconcisegamerockthere

Problem

I am learning JavaScript and I just wrote a Rock, Paper, Scissors game that will first prompt the two players for their choices and validate them. In the event of a tie, each player will choose their answer again and it will once again be validated. Everything works fine, but it feels like I have a lot of repetition going on here. Since I'm new to this, I can't figure out a way to make this more concise if possible.

var compare = function(choice1, choice2){
    while (choice1 != 'rock' && choice1 != 'paper' && choice1 != "scissors"){
        choice1 = prompt('Invalid entry. Player 1, please choose rock, paper, or scissors only');
    }    
    while (choice2 != 'rock' && choice2 != 'paper' && choice2 != "scissors"){
        choice2 = prompt('Invalid entry. Player 2, please choose rock, paper, or scissors only');
    }
    while (choice1 == choice2){
        choice1 = prompt('Tiebreaker!. Player 1, please choose rock, paper, or scissors only');
     while (choice1 != 'rock' && choice1 != 'paper' && choice1 != "scissors"){
        choice1 = prompt('Invalid Entry. Player 1, please choose rock, paper, or scissors only');
        }
        choice2 = prompt('Tiebreaker!. Player 2, please choose rock, paper, or scissors only');
     while (choice2 != 'rock' && choice2 != 'paper' && choice2 != "scissors"){
        choice2 = prompt('Invalid Entry. Player 2, please choose rock, paper, or scissors only');
        }

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

Solution

Just build an object with the associativity.

// what wins to what
var winsTo = {
  'rock': 'scissors',
  'paper': 'rock',
  'scissors': 'paper',
};

// invalid selected
if (!(choice1 in winsTo && choice2 in winsTo)) {
  alert("bad choice !");
  return;
}

// same choice => equality
if (choice1 == choice2) {
  alert("Equality!");
} else if (winsTo[choice1] == choice2) {
  alert("choice1 won!");
} else {
  alert("choice2 won!");
}

Code Snippets

// what wins to what
var winsTo = {
  'rock': 'scissors',
  'paper': 'rock',
  'scissors': 'paper',
};

// invalid selected
if (!(choice1 in winsTo && choice2 in winsTo)) {
  alert("bad choice !");
  return;
}

// same choice => equality
if (choice1 == choice2) {
  alert("Equality!");
} else if (winsTo[choice1] == choice2) {
  alert("choice1 won!");
} else {
  alert("choice2 won!");
}

Context

StackExchange Code Review Q#27225, answer score: 11

Revisions (0)

No revisions yet.