patternjavascriptMinor
Rock-Paper-Scissors with the revealing module pattern
Viewed 0 times
scissorspaperthewithmodulerevealingrockpattern
Problem
In my ongoing effort in learning advanced JavaScript I tried to develop the traditional rock paper scissors game by using the famous revealing module pattern. I'm not comfortable yet with what has to be Private/Public. In my solution, I decided to have a single public function:
Anyway I commented my code a lot I hope it will explain my intent correctly. I'm looking for any advices I can get, even if not directly related to the pattern I used.
```
var roshambo = (function() {
// We will add +1 for each human victory
var privateGameDB = 0;
// We will add +1 for each human or robot victory
var privateGameCount = 0;
// Get human input with validation, returns a number between 1 and 3
function privateGetInput() {
var choice = prompt('Rock: 1, Paper: 2, Scissors: 3');
// Input validation
while (parseInt(choice) = 4 || isNaN(parseInt(choice))) {
choice = prompt('Rock: 1, Paper: 2, Scissors:3 (your choice vas not valid)');
}
return parseInt(choice);
}
// Generate robot's play, randomize a number between 1 and 3
// Also includes a display box alerting the robot's play
function privateGetRobotPlay() {
var play = Math.floor(Math.random() * 3) + 1;
switch (play) {
case 1:
alert('Robot played Rock');
break;
case 2:
alert('Robot played Paper');
break;
case 3:
alert('Robot played Scissors');
break;
}
return play;
}
// Check who wins the round, by comparing the two numbers.
// Returns 0 on draw, 1 on loss and 2 on win
function privateComparePlays(human, robot) {
var humanPlay = human;
var robotPlay = robot;
// Tie
if (robotPlay == humanPlay) {
return 0;
}
// Robot wins
if (robotPlay == 1 && humanPlay == 3 || robot
Game. It uses a lot of private stuff and organizes the global game behavior. Everything else is private and not available to the caller. I'm not sure if it's a correct use of that pattern ?Anyway I commented my code a lot I hope it will explain my intent correctly. I'm looking for any advices I can get, even if not directly related to the pattern I used.
```
var roshambo = (function() {
// We will add +1 for each human victory
var privateGameDB = 0;
// We will add +1 for each human or robot victory
var privateGameCount = 0;
// Get human input with validation, returns a number between 1 and 3
function privateGetInput() {
var choice = prompt('Rock: 1, Paper: 2, Scissors: 3');
// Input validation
while (parseInt(choice) = 4 || isNaN(parseInt(choice))) {
choice = prompt('Rock: 1, Paper: 2, Scissors:3 (your choice vas not valid)');
}
return parseInt(choice);
}
// Generate robot's play, randomize a number between 1 and 3
// Also includes a display box alerting the robot's play
function privateGetRobotPlay() {
var play = Math.floor(Math.random() * 3) + 1;
switch (play) {
case 1:
alert('Robot played Rock');
break;
case 2:
alert('Robot played Paper');
break;
case 3:
alert('Robot played Scissors');
break;
}
return play;
}
// Check who wins the round, by comparing the two numbers.
// Returns 0 on draw, 1 on loss and 2 on win
function privateComparePlays(human, robot) {
var humanPlay = human;
var robotPlay = robot;
// Tie
if (robotPlay == humanPlay) {
return 0;
}
// Robot wins
if (robotPlay == 1 && humanPlay == 3 || robot
Solution
I like your question,
-
I think if you are going to export one function, and then immediately execute it, then you are better served with a self executing query.
-
You are writing a game, I would use a revealing pattern for the model, the controller, and the view.
-
Always consider if you can replace control structures with data structures:
instead of
For extra points, consider how you can convert this into a data structure:
-
Consider wellnamed variables or constants instead of magic numbers, your code would be so much more readable if you had upfront
and then later
of course in this particular case, I would not have gone for a case but an
-
I think if you are going to export one function, and then immediately execute it, then you are better served with a self executing query.
-
You are writing a game, I would use a revealing pattern for the model, the controller, and the view.
- View would export
- An alert function
- An input function
- Controller would export a
- init or start function
- It would have a lot of internal functions calling view and model
- Model would export a
- Getter/setter for victories
- comparePlays function
-
Always consider if you can replace control structures with data structures:
// Display a message depending on round result
function privateAlertRoundResults(results) {
var resultsMessages = [ 'Draw !', 'You lose !','You win !'];
alert( resultsMessages[ results ];
}instead of
// Display a message depending on round result
function privateAlertRoundResults(results) {
switch (results) {
case 0:
alert('Draw !');
break;
case 1:
alert('You lose !');
break;
case 2:
alert('You win !');
break;
}
}For extra points, consider how you can convert this into a data structure:
if (robotPlay == 1 && humanPlay == 3 || robotPlay == 2 && humanPlay == 1 || robotPlay == 3 && humanPlay == 2) {-
Consider wellnamed variables or constants instead of magic numbers, your code would be so much more readable if you had upfront
var DRAW = 0,
LOSE = 1,
WIN = 2;and then later
function privateUpdateDB(check) {
switch (check) {
case LOSE:
privateGameCount++;
break;
case WIN:
privateGameDB++;
privateGameCount++;
break;
}
}of course in this particular case, I would not have gone for a case but an
if:function privateUpdateDB(check) {
if( check == LOSE ){
privateGameCount++;
} else if ( check == WIN ){
privateGameDB++;
privateGameCount++;
}
//We don't increase privateGameCount for Draws (?)
}- I like your adjusting prompt in case the user provides a bad number
- On the whole, prefixing your private functions with
privateis not a great idea because it interrupts the reading flow. I would not prefix at all, but if you insist on prefixing I would prefix with_, most developers will know what you mean (and frown because they also don't like prefixing ;)
Code Snippets
// Display a message depending on round result
function privateAlertRoundResults(results) {
var resultsMessages = [ 'Draw !', 'You lose !','You win !'];
alert( resultsMessages[ results ];
}// Display a message depending on round result
function privateAlertRoundResults(results) {
switch (results) {
case 0:
alert('Draw !');
break;
case 1:
alert('You lose !');
break;
case 2:
alert('You win !');
break;
}
}if (robotPlay == 1 && humanPlay == 3 || robotPlay == 2 && humanPlay == 1 || robotPlay == 3 && humanPlay == 2) {var DRAW = 0,
LOSE = 1,
WIN = 2;function privateUpdateDB(check) {
switch (check) {
case LOSE:
privateGameCount++;
break;
case WIN:
privateGameDB++;
privateGameCount++;
break;
}
}Context
StackExchange Code Review Q#134719, answer score: 5
Revisions (0)
No revisions yet.