2.0/10
verdict: needs_serious_help
"Nested IF hell with rock-paper-scissors logic"
lang: JavaScript·40 lines
//your_submission
RockPaperScissorsAbomination.js
function playRockPaperScissors(player1, player2) {
if (player1 === 'rock') {
if (player2 === 'rock') {
return 'Draw';
} else if (player2 === 'paper') {
return 'Player 2 wins';
} else if (player2 === 'scissors') {
return 'Player 1 wins';
} else {
return 'Invalid move P2';
}
} else if (player1 === 'paper') {
if (player2 === 'rock') {
return 'Player 1 wins';
} else if (player2 === 'paper') {
return 'Draw';
} else if (player2 === 'scissors') {
return 'Player 2 wins';
} else {
return 'Invalid move P2';
}
} else if (player1 === 'scissors') {
if (player2 === 'rock') {
return 'Player 2 wins';
} else if (player2 === 'paper') {
return 'Player 1 wins';
} else if (player2 === 'scissors') {
return 'Draw';
} else {
return 'Invalid move P2';
}
} else {
return 'Invalid move P1';
}
}
const p1 = 'rock';
const p2 = 'scissors';
const result = playRockPaperScissors(p1, p2);
console.log('Winner: ' + result);//detailed_analysis
!
Nested IF Statement Apocalypse
The author apparently thought that 9 levels of nested IF statements would be a good idea. Newsflash: it's not. This is a maintainability nightmare.
?
No Input Validation (Sort Of)
While the code does check for invalid moves, it does so in a way that's both inefficient and hard to read. A simple array of valid moves would have sufficed.
!
Magic Strings Everywhere
The code is littered with magic strings ('rock', 'paper', 'scissors'). This is a recipe for disaster. Define constants or enums, for the love of all things good.
//suggested_fix
RockPaperScissorsAbomination_fixed.js
+function playRockPaperScissors(player1, player2) {
+ const moves = ['rock', 'paper', 'scissors'];
+ if (!moves.includes(player1) || !moves.includes(player2)) {
+ return 'Invalid move';
+ }
+ if (player1 === player2) {
+ return 'Draw';
+ }
+ if ((player1 === 'rock' && player2 === 'scissors') ||
+ (player1 === 'scissors' && player2 === 'paper') ||
+ (player1 === 'paper' && player2 === 'rock')) {
+ return 'Player 1 wins';
+ }
+ return 'Player 2 wins';
+}
+const p1 = 'rock';
+const p2 = 'scissors';
+const result = playRockPaperScissors(p1, p2);
+console.log('Winner: ' + result);