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

How do I make my Rock-Paper-Scissor program more JavaScript-like?

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

Problem

I have implemented my first program in JavaScript, Rock-Paper-Scissors. I would like JavaScript developers to help me make this program more JavaScript-like by following the idioms of the language. I am also open to any suggestions related to simplification of logic and code readability.

The following is the complete listing of the program:


    
        var choiceArray = ["paper", "rock", "scissors"];

        var getUserChoice = function()
        {
            var userChoice = undefined;
            var userChoiceIndex = undefined;

            while (true)
            {
                userChoice = prompt("What do you like - paper, scissor or rock");
                userChoiceIndex = choiceArray.indexOf(userChoice);

                if (userChoiceIndex >= 0 && userChoiceIndex 

Solution

Well, there might be a few things.

The most important thing that can be changed is removing all those magic numbers: What happens when we want to play “Rock, Paper, Scissors, Lizard, Spock” instead? You'd think you would only have to update the choiceArray – but unfortunately the length of that array is encoded in various places.

This means that if (userChoiceIndex >= 0 && userChoiceIndex <= 2) should be

if (userChoiceIndex >= 0 && userChoiceIndex <= choiceArray.length - 1)


and var computerChoiceIndex = Math.floor(Math.random() * 3) should be

var computerChoiceIndex = Math.floor(Math.random() * choiceArray.length);


Your decideWinner function isn't really extensible, because you encode the winning relationships as code, not as the data it actually is. I'd use an object literal that only encodes what choice wins over another, like:

var winningData = {
  paper:     { rock:     true },
  rock:      { scissors: true },
  scissors:  { paper:    true }
};


Also, I'd implement decideWinner as a comparator that returns one of -1, 0, 1 depending on whether the first or the second argument won. This decouples the decision logic from UI like text.

function(choiceA, choiceB) {
  if (choiceA === choiceB)           return  0;
  if (winningData[choiceA][choiceB]) return -1;
  if (winningData[choiceB][choiceA]) return  1;
  console.warn("No ordering between " + choiceA + " and " + choiceB + " was found");
  return 0;
}


The game result is then determined as:

var ordering = decideWinner(userChoice, computerChoice);
var winningMessage = ordering  0 ? "The computer wins!" :
                                    "It's a tie!";
var gameResult = "You chose: "          + userChoice     + ".\n" +
                 "The computer chose: " + computerChoice + ".\n" +
                 winningMessage;


Now when we extend the game, we just have to update the winningData. Notice that I display who wins, not what choice wins, as I don't think that would be tremendously interesting to a player. Also, he would have to make an association from the choice to the player, which is unnecessary mental effort.

Code Snippets

if (userChoiceIndex >= 0 && userChoiceIndex <= choiceArray.length - 1)
var computerChoiceIndex = Math.floor(Math.random() * choiceArray.length);
var winningData = {
  paper:     { rock:     true },
  rock:      { scissors: true },
  scissors:  { paper:    true }
};
function(choiceA, choiceB) {
  if (choiceA === choiceB)           return  0;
  if (winningData[choiceA][choiceB]) return -1;
  if (winningData[choiceB][choiceA]) return  1;
  console.warn("No ordering between " + choiceA + " and " + choiceB + " was found");
  return 0;
}
var ordering = decideWinner(userChoice, computerChoice);
var winningMessage = ordering < 0 ? "You win!" :
                     ordering > 0 ? "The computer wins!" :
                                    "It's a tie!";
var gameResult = "You chose: "          + userChoice     + ".\n" +
                 "The computer chose: " + computerChoice + ".\n" +
                 winningMessage;

Context

StackExchange Code Review Q#33314, answer score: 5

Revisions (0)

No revisions yet.