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

Rock Paper Scissors in JavaScript

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

Problem

In a few weeks I need to give a workshop about JavaScript to colleagues. They do not have much experience with JavaScript so I will explain the basics of JavaScript. At the end of the day I want them to make a small game with the aspects learned during the day.

I was thinking of Rock Paper Scissors. Below you can find the final code for this game:

var RockPaperScissors = (function () {
    var t = 'Tie',
        c = 'Computer wins',
        p = 'Player wins',
        winningMap = [
            [t, c, p],
            [p, t, c],
            [c, p, t]
        ],
        choices = ["Rock", "Paper", "Scissors"];

    var getComputerChoice = function () {
        return Math.floor(Math.random() * 3);
    };

    var getWinner = function (playerChoice, computerChoice) {
        return winningMap[playerChoice][computerChoice];
    };

    function RockPaperScissors(playerChoice) {
        this.playerChoice = playerChoice;
        this.computerChoice = getComputerChoice();
        this.winner = getWinner(this.playerChoice, this.computerChoice);
    }

    RockPaperScissors.prototype.toString = function () {
        return this.winner + " [Computer: " + choices[this.computerChoice] + ", Player: " + choices[this.playerChoice] + "]";
    };

    return RockPaperScissors;

})();


This code is executed everytime a select input box changes (see this as the user input):

$('#choice').on('change', function () {
    var $input = $(this);

    if ($input.val() === '-1') return;

    var rockPaperScissors = new RockPaperScissors($input.val());

    $('#message').html(rockPaperScissors.toString());
});


I've also set up a Fiddle.

Do you think this code is clear for beginners? Is this too advanced? I'm not sure if I should introduce them to prototyping or not.

Solution

The definition of the winningMap is very confusing.

I suggest a different form of input:

var winningMap = {Rock: "Scissors", Paper: "Rock", Scissors: "Paper"};


It defines which choice can be beaten by a specific selection.
If both inputs are the same, it's obvious a tie:

var getWinner = function (playerChoice, computerChoice) {
    if(playerChoice === computerChoice){
        return "Tie";
    }
    if(computerChoice === winningMap[playerChoice]){
        return "Player Wins";
    }
    return "Computer Wins";
};


Notice, that I didn't use the string-definitions. I don't think that it's necessary, and it only makes the code a bit more complicated.

In order to convert the computer's choice to an actual string (Rock, Scissors, Paper), you can use an Array:

var stringMapping = ["Rock", "Scissors", "Paper"];

var getComputerChoice = function () {
    return stringMapping[ Math.floor(Math.random() * 3) ];
};


Maybe a new-Operator would be understood easier

The closure might be confusing for a beginner, maybe you should consider to write the Object this way:

function RockPaperScissors (){ 
     var getComputerChoice = function(){ ... };    //= private variable
     this.getWinner = function(){ ... };           //= public variable
};

var actualObject = new RockPaperScissors();
actualObject.method();
actualObject['method']();


If your colleagues are used to object oriented programming, this style might be more familiar.

Always use brackets

if ($input.val() === '-1') { return; }


I personally don't mind conditions within one line, but dropping the brackets is bad practice - and can lead to strange behaviours, especially in JavaScript.

Furthermore, I wonder why u used the dollar-sign at the beginning of the $input-variable?

Edit:

I share @Barts's opinion from the comments - it's not neccessary to use jQuery in this example. You should show them jQuery once they know the basic principles of JavaScript.

Code Snippets

var winningMap = {Rock: "Scissors", Paper: "Rock", Scissors: "Paper"};
var getWinner = function (playerChoice, computerChoice) {
    if(playerChoice === computerChoice){
        return "Tie";
    }
    if(computerChoice === winningMap[playerChoice]){
        return "Player Wins";
    }
    return "Computer Wins";
};
var stringMapping = ["Rock", "Scissors", "Paper"];

var getComputerChoice = function () {
    return stringMapping[ Math.floor(Math.random() * 3) ];
};
function RockPaperScissors (){ 
     var getComputerChoice = function(){ ... };    //= private variable
     this.getWinner = function(){ ... };           //= public variable
};

var actualObject = new RockPaperScissors();
actualObject.method();
actualObject['method']();
if ($input.val() === '-1') { return; }

Context

StackExchange Code Review Q#54240, answer score: 13

Revisions (0)

No revisions yet.