A cleaner way to write this block of JS code

Question

I'm following a JS course for beginners and the first lesson was about making a "rock, paper, scissors" game.

The instructor said something about this not being the most efficient way to determine the winner but didn't really touch on how to write it. All I can think of is using Switch statement instead of if.

Can someone show me how it should be done?

function getResult(){
    if (computerChoice === userChoice) {
        result = "It's a draw";
    } else if (computerChoice == "rock" && userChoice == "scissors") {
        result = 'You lost';
    } else if (computerChoice == "rock" && userChoice == "paper") {
        result = 'You won!';
    } else if (computerChoice == "paper" && userChoice == "scissors") {
        result = 'You won';
    } else if (computerChoice == "paper" && userChoice == "rock") {
        result = 'You lost';
    } else if (computerChoice == "scissors" && userChoice == "rock") {
        result = 'You won';
    } else if (computerChoice == "scissors" && userChoice == "paper") {
        result = 'You lost';
    }
    resultDisplay.innerHTML=result;
}

Thanks, Amin

`result = ["It's a draw", "You lost", "You won"][Math.floor(Math.random() * 3)]` — Phil, Jul 18 '22 at 04:54
Does this answer your question? [Switch statement for multiple cases in JavaScript](https://stackoverflow.com/questions/13207927/switch-statement-for-multiple-cases-in-javascript) — zircon, Jul 18 '22 at 05:20

score 2 · Answer 1 · answered Jul 18 '22 at 04:57

If you associate a number with each possible choice, you can then subtract them from each other. If it isn't a tie, there are only 2 possible options left (after modulo), which allows you to concisely determine who won.

const choices = ['rock', 'paper', 'scissors'];
for (let i = 0; i < 3; i++) {
  for (let j = 0; j < 3; j++) {
    console.log(getResult(choices[i], choices[j]));
  }
}

function getResult(computerChoice, userChoice){
    const desc = `Comp: ${computerChoice}, you: ${userChoice}: `;
    if (computerChoice === userChoice) {
        return desc + "It's a draw";
    }
    const diff = (3 + choices.indexOf(computerChoice) - choices.indexOf(userChoice)) % 3;
    return desc + (diff === 2 ? 'You won' : 'You lost');
}

samnoon · Answer 2 · 2022-07-18T05:16:27.483

You can create an object that defines if your move is weak or strong against another.

See bellow code:

function getResult() {
  const weapons = {
    rock: {
      weakTo: 'paper',
      strongTo: 'scissors'
    },
    paper: {
      weakTo: 'scissors',
      strongTo: 'rock'
    },
    scissors: {
      weakTo: 'rock',
      strongTo: 'paper'
    }
  }

  if (weapons[userChoice].strongTo === computerChoice) {
    result = "You Won";
  } else if (weapons[userChoice].weakTo === computerChoice) {
    result = "You lost";
  } else {
    result = "It's a draw";
  }

  resultDisplay.innerHTML = result;
}

score 0 · Accepted Answer · edited Jul 20 '22 at 03:50

When solving a problem like this, it's good to think about your data model first. How would you create a data structure that made it easy to answer the question you need to answer?

Maybe something like:

const winsVs = {
    rock: 'scissors',
    paper: 'rock',
    scissors: 'paper'
}

Now winsVs.rock === 'scissors' which means that rock wins versus scissors.

Now you have an if condition with just the three cases: tie, win and lose.

const winsVs = {
    rock: 'scissors',
    paper: 'rock',
    scissors: 'paper'
}

function getResult(computerChoice, userChoice) {
    let result
    if (computerChoice === userChoice) {
        result = "It's a draw"
    } else if (winsVs[computerChoice] === userChoice) {
        result = "You lost"
    } else if (winsVs[userChoice] === computerChoice) {
        // could also be just `else {` since only one option is left.
        result = "You won"
    }
    return result
}

console.log(getResult('paper', 'paper')) // It's a draw
console.log(getResult('rock', 'paper')) // You won
console.log(getResult('paper', 'rock')) // You lost

score 0 · Answer 4 · answered Jul 18 '22 at 05:14

One way to optimise this would be to remove else statements, and set resultDisplay.innerHTML in the place getResult() is being called.

function getResult(){
    if (computerChoice === userChoice) {
        return "It's a draw";
    }
    if (computerChoice === "rock") {
        if (userChoice === "scissors") {
            return 'You lost';
        }
        if (userChoice === "paper") {
            return 'You won!';
        }
    } else if (computerChoice === "paper") {
        if (userChoice === "scissors") {
            return 'You won';
        }
        if (userChoice === "rock") {
            return 'You lost';
        }
    } else if (computerChoice === "scissors") {
        if (userChoice === "rock") {
            return 'You lost';
        }
        if (userChoice === "paper") {
            return 'You won';
        }
    }

    return '';
}

Calling getResult():

resultDetail.innerHTML = getResult();

A cleaner way to write this block of JS code

4 Answers4