8
\$\begingroup\$

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.

\$\endgroup\$
7
  • \$\begingroup\$ 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. \$\endgroup\$
    – maja
    Commented Jun 14, 2014 at 12:03
  • \$\begingroup\$ And maybe a new-Operator would be understood easier instead of the closure? var RockPaperScissors = new function(){ ... }; \$\endgroup\$
    – maja
    Commented Jun 14, 2014 at 12:06
  • \$\begingroup\$ @maja - why don't you combine your thoughts in to an actual answer... ? \$\endgroup\$
    – rolfl
    Commented Jun 14, 2014 at 12:09
  • 2
    \$\begingroup\$ I guess "too advanced" would depend on what your audience already knows. What other languages do they usually code? Or are they complete beginnings, because in that case you should probably start with "Hello, world" (or at least be prepared to start there). \$\endgroup\$
    – Flambino
    Commented Jun 14, 2014 at 12:47
  • 5
    \$\begingroup\$ I don't see why you would require jQuery for a workshop on javascript. I mean... $input.val() vs input.value? If I knew nothing of javascript, I would pick option 2... \$\endgroup\$
    – Bart
    Commented Jun 14, 2014 at 13:49

1 Answer 1

13
\$\begingroup\$

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.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ +1 for the "use brackets/braces" line. Better to learn the stricter syntax rules-of-thumb first; you'll learn the shortcuts later. \$\endgroup\$
    – Flambino
    Commented Jun 14, 2014 at 12:40
  • \$\begingroup\$ Oh, the +1 is also for all the other points, but the braces is a pet-peeve of mine :) \$\endgroup\$
    – Flambino
    Commented Jun 14, 2014 at 13:07
  • \$\begingroup\$ Thank you for your input. The winningMap is definitely better like this. I know there are many ways to write Javascript objects. During the workshop I will briefly explain scoping in Javascript. Since they are experienced C# backend developers this will be the most confusing part and has to be explained into detail. I'm going to keep the closure as is because they will also get a course about Typescript. My colleague will explain them how Typescript compiles to Javascript. So they need to have an understanding of scoping & closures. \$\endgroup\$
    – Dieterg
    Commented Jun 14, 2014 at 15:37
  • \$\begingroup\$ I use the $-sign to indicate I'm working with a jQuery element. It's just my prefered way of working with jQuery. \$\endgroup\$
    – Dieterg
    Commented Jun 14, 2014 at 17:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.