Feedback on rock paper scissors implementation

Having completed the course, I’ve revisited some of the freeform exercises and looking at how I could use learnings to improve (maybe?) the implementation.

My revised code for https://www.codecademy.com/courses/introduction-to-javascript/projects/rock-paper-scissors-javascript is below.

If anyone has time, I’d like some critical feedback on it.

const availableInputs = ['rock', 'paper', 'scissors'];
const tiedGame = 'The game is tied';
const userWin = 'Congratulations, you won!';
const computerWin = 'Sorry, you lost!';
const invalidUserInputError = `You must select one of:${availableInputs.map(input => `\n- ${input}`)}`

function getUserChoice(userInput) {
  userInput = userInput.toLowerCase();
  if (availableInputs.includes(userInput) || userInput === 'bomb') {
    return userInput;
  } else {
    console.log(invalidUserInputError)
  }
};

function getComputerChoice() {
  let computerChoice = Math.floor(Math.random() * 3);
  return availableInputs[computerChoice];
};

function determineWinner(userChoice, computerChoice) {
  if (userChoice === 'bomb') {
    return userWin;
  } else if (userChoice === computerChoice) {
    return tiedGame
  } else {  
    switch(userChoice) {
      case 'rock':
        return computerChoice === 'paper' ? computerWin : userWin;
        break;
      case 'paper':
        return computerChoice === 'scissors' ? computerWin : userWin;
        break;
      case 'scissors':
        return computerChoice === 'rock' ? computerWin : userWin;
        break;
    }
  }
}

function playGame(userChoice) {
  let outcome = determineWinner(getUserChoice(userChoice), getComputerChoice())
  console.log(outcome);
}

let times = 30
while (times > 0) {
  playGame('Rock');
  times--;
}

Helo, @systemrunner39128.

Looks good. One thing I’d change though is the break; statements following the return ... statements. They are unnecessary, and will never be reached. Once a return statement is executed, control is passed back to the ‘caller’. Using return instead of break in a switch ... case is perfectly acceptable.

You could also include case 'bomb': in your switch instead of having a special if statement to check for it.

One other suggestion, since you’ve gone this far, would be to add a function to randomly select the userChoice as well, so when you play 30 games, the userChoice will vary. I might also consider logging the computerChoice and userChoice as part of the outcome. Something along the lines of:

You played paper.
Computer played rock.
Congratulations, you won!

Nice, thanks for the feedback!!

I’ve implemented the changes apart from randomising user input.

The implicit versus explicit return in a case statement is interesting, the course doesn’t really cover it off, but I’m approaching JS from a ruby through osmosis background where the implicit return is normal. and doesn’t require anything special. Good to know that in JS the explicit return doesn’t require the break statement :+1:

1 Like