snippetjavascriptMinor
How do I make my Rock-Paper-Scissor program more JavaScript-like?
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:
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
This means that
and
Your
Also, I'd implement
The game result is then determined as:
Now when we extend the game, we just have to update the
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 bevar 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.