patternjavascriptModerate
Rock Paper Scissors in JavaScript
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:
This code is executed everytime a select input box changes (see this as the user input):
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.
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:
It defines which choice can be beaten by a specific selection.
If both inputs are the same, it's obvious a tie:
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 (
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:
If your colleagues are used to object oriented programming, this style might be more familiar.
Always use brackets
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
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.
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.