Rock, Paper, Scissors in JavaScript

Question

I'm learning javascript and want to make my code as efficient as possible. This is something I wrote a while back and I keep revising it as I learn new things. But since I'm partly OCD, I can't help but want to stop repeating myself (DRY).

Can someone please help me use Vanilla JS to make this cleaner?

Here is the final code at CodePen!

jsFiddle

// Start All the Variables //
var userChoice;
var watson;
var saying = "You chose " + userChoice + ". Watson chose " + watson;
var win = " You Win!";
var lose = " You Lose";
var tie = " We Tied";

var audioWinner = new Audio('http://facebook.design/public/sounds/Success 3.mp3');
var audioLoser = new Audio('http://facebook.design/public/sounds/Error 1.mp3');
var audioTied = new Audio('http://facebook.design/public/sounds/Collapse.mp3');

var winner = document.getElementById('winner');
var loser = document.getElementById('loser');
var tied = document.getElementById('tied');
var rock = document.getElementById("rock"); // rock button el
var paper = document.getElementById("paper"); // paper button el
var scissors = document.getElementById("scissors"); // scissors button el
var myChoice = document.getElementById("myChoice"); // user choice el
var cpuChoice = document.getElementById("cpuChoice"); // watson choice el
var verdict = document.getElementById("verdict"); // verdict el

// USER'S CHOICE
// *************
rock.addEventListener("click", function(){
  userChoice = 0;
  myChoice.innerHTML = "You chose: Rock";
  checkWatson();
  compare();
});
paper.addEventListener("click", function(){
  userChoice = 1;
  myChoice.innerHTML = "You chose: Paper";
  checkWatson();
  compare();
});
scissors.addEventListener("click", function(){
  userChoice = 2;
  myChoice.innerHTML = "You chose: Scissors";
  checkWatson();
  compare();
});

// WATSON'S CHOICE
// ***************
function checkWatson() {
  // generates a random number between 0-2 
  randomNum = Math.floor(Math.random() * 3);

  // generate a random number and assign it to one of the 3 choices
  if (randomNum === 0) { 
    watson = "rock";
  } else if (randomNum === 1) { 
    watson = "paper";
  } else { 
    watson = "scissors";
  }
  console.log('Watson chose: ' + watson);  
}

// 3 OUTCOME FUNCTIONS
// *******************
function resultsTie() {
  audioTied.play();
  verdict.innerHTML = tie; // tie
  winner.style.display = 'none';
  loser.style.display = 'none';
  tied.style.display = 'block';
}
function resultsWinner() {
  audioWinner.play();
  verdict.innerHTML = win; // win
  winner.style.display = 'block';
  loser.style.display = 'none';
  tied.style.display = 'none';
}
function resultsLoser() {
  audioLoser.play();
  verdict.innerHTML = lose;
  winner.style.display = 'none';
  loser.style.display = 'block';
  tied.style.display = 'none';
}

// COMPARE USER VS WATSON
// **********************
function compare() {
  // user chooses rock
  if (userChoice ===  randomNum) { 
    resultsTie();
  } else if (userChoice === 0 && randomNum === 1) { 
    resultsLoser();
  } else if (userChoice === 0 && randomNum === 2) { 
    resultsWinner();
  }

  // user chooses paper  
  if (userChoice === 1 && randomNum === 0) { 
    resultsWinner();
  }
  else if (userChoice === 1 && randomNum === 2) { 
    resultsLoser();
  }

  // user chooses scissors
  if (userChoice === 2 && randomNum === 0) { 
    resultsLoser();
  } else if (userChoice === 2 && randomNum === 1) { 
    resultsWinner();
  }

  cpuChoice.innerHTML = "Watson chose: " + watson;
}

<img class="icon" id="rock" src="https://png.icons8.com/rock/win8/64" title="Rock" width="50" height="50">
<img class="icon" id="paper" src="https://png.icons8.com/paper-filled/ios7/50" title="Paper Filled" width="50" height="50">
<img class="icon" id="scissors" src="https://png.icons8.com/scissors-filled/ios7/50" title="Scissors Filled" width="50" height="50">

<div class="wrapper">
  <p id="output"></p>
</div>

<div style="border: 0px solid blue; display: block;">
  <p id="myChoice"></p>
  <p>- VS -</p>
  <p id="cpuChoice"></p>
  <hr>
  <p id="verdict"></p>
</div>

<div id="winner">😁</div>
<div id="loser">😫</div>
<div id="tied">🙈</div>

I made this game for my SON, and he loves it BTW :)

Answer 1

Drying out the code.

You don't need OCD to want DRY code. It is a fundamental requirement of any good code (with a few special case exceptions) that it is DRY.

Looking at the code and first impression is that its "OMG bottom of the pool soaking wet."

How to dry code.

Look for repeating code. Yep obvious I know but look at the following...

var audioWinner = new Audio('http://facebook.design/public/sounds/Success 3.mp3');
var audioLoser = new Audio('http://facebook.design/public/sounds/Error 1.mp3');
var audioTied = new Audio('http://facebook.design/public/sounds/Collapse.mp3');

var winner = document.getElementById('winner');
var loser = document.getElementById('loser');
var tied = document.getElementById('tied');
var rock = document.getElementById("rock"); // rock button el
var paper = document.getElementById("paper"); // paper button el
var scissors = document.getElementById("scissors"); // scissors button el
var myChoice = document.getElementById("myChoice"); // user choice el
var cpuChoice = document.getElementById("cpuChoice"); // watson choice el
var verdict = document.getElementById("verdict"); // verdict el

Even the comments repeat, there are 12 var ,3 new audio and http://facebook.design/public/sounds, and 9 document.getElementById(

I removed only the relevant content and counted the characters. ~80% of the the above code is redundant. with only 12 unique values in there, the filenames of the audio, and the element ids, which you repeat again in the rest of the code, and HTML.

I don't think you know, but What makes it even more not dry is that every modern browser already has all the variables names created for you. Each line var winner = document.getElementById('winner'); is simply replacing the existing global variable. Removing all the get element lines and your code would run just as well.

---

Functions

If you write many functions with just a few minor differences its a good sign that you can write one function and pass the differences as arguments.

Each function has just two differences, and abstractly just one "The choice"

rock.addEventListener("click", function(){
  userChoice = 0;
  myChoice.innerHTML = "You chose: Rock";
  checkWatson();
  compare();
});
paper.addEventListener("click", function(){
  userChoice = 1;
  myChoice.innerHTML = "You chose: Paper";
  checkWatson();
  compare();
});
scissors.addEventListener("click", function(){
  userChoice = 2;
  myChoice.innerHTML = "You chose: Scissors";
  checkWatson();
  compare();
});

Each of the following also have just one abstract difference the result.

// 3 OUTCOME FUNCTIONS
// *******************
function resultsTie() {
  audioTied.play();
  verdict.innerHTML = tie; // tie
  winner.style.display = 'none';
  loser.style.display = 'none';
  tied.style.display = 'block';
}
function resultsWinner() {
  audioWinner.play();
  verdict.innerHTML = win; // win
  winner.style.display = 'block';
  loser.style.display = 'none';
  tied.style.display = 'none';
}
function resultsLoser() {
  audioLoser.play();
  verdict.innerHTML = lose;
  winner.style.display = 'none';
  loser.style.display = 'block';
  tied.style.display = 'none';
}

Each group of three can easily be replaced with one. The click functions can use the click event to get the needed information from the DOM, and the result functions can be passed an object that contain the sound, message and element to display the message.

---

Logic

Look for self similar logic. The following function has but 3 outcomes, with only two input values. but each input is referenced at least 3 times and at worst 7 times each. Even after you have displayed a result you still test for results that you already know are not true.

function compare() {
  // user chooses rock
  if (userChoice ===  randomNum) { 
    resultsTie();
  } else if (userChoice === 0 && randomNum === 1) { 
    resultsLoser();
  } else if (userChoice === 0 && randomNum === 2) { 
    resultsWinner();
  }

  // user chooses paper  
  if (userChoice === 1 && randomNum === 0) { 
    resultsWinner();
  }
  else if (userChoice === 1 && randomNum === 2) { 
    resultsLoser();
  }

  // user chooses scissors
  if (userChoice === 2 && randomNum === 0) { 
    resultsLoser();
  } else if (userChoice === 2 && randomNum === 1) { 
    resultsWinner();
  }

---

Data types

I don't mean to repeat my self with the obvious. BUT..

Dont repeat across data types. You have names in HTML and names in Javascript, though not always possible you can often use the data from one to provide the needed info for another. For example the id of the user clickable images "rock, paper, scissors" contain the info you need to name the hands.

---

DRY but not a desert.

It is not possible to be without repeating code, sometimes the complexity of the problem makes it hard to avoid repetition, or repetition may be the quickest way to solve the problem when time is short. Sometimes when execution performance is the driving force repetition is the only way to get the best performance.

The length some coders go to too avoid repeating code can become so complex and difficult to follow that they have lost any advantage they would get from DRY code. A DRY KISS is best.

DRY is practical, saves you time entering and modifying code. DRY code is more compact and thus should be easier to read and understand. If this does not happen then it was best left as it was.

---

Example

Your code was not bad, and the changes I would have made if DRY was not the considering option would have been just the win logic and remove the repeated click and end game functions.

Your code after a little drying out.

var userChoice, watson;

// array of strings in order [name of play, sound file, play result message]
const setup = [ 
    ["winner", "Success 3", " You Win!"],  
    ["loser",  "Error 1",   " You Lose"],
    ["tied",   "Collapse",  " We Tied"]
];

// Set up the 3 results types 
const plays = {};
setup.forEach(play => {
    plays[play[0]] = {
        audio : new Audio(`http://facebook.design/public/sounds/${play[1]}.mp3`),
        message : play[2],
        element : document.querySelector("#"+play[0]),
    };            
});

// Handles the player hand selection.
buttons.addEventListener("click", event => {
   userChoice =event.target.id;
   myChoice.textContent = "You pick : " + userChoice;
   watson = (["paper","scissors","rock"])[Math.random() * 3 | 0];
   cpuChoice.textContent = "Watson chose: " + watson;
   compare();

});


const results = { // names contain player choice then computer choice.
    rock : {                     // play picked rock
        paper : plays.loser,     // computer paper
        scissors : plays.winner, // computer scissor      
    },                           // no need for draw condition.
    paper : {
        rock : plays.winner,
        scissors : plays.loser, 
    },
    scissors : {
        rock : plays.loser,
        paper :  plays.winner,  
    } 
}


function displayResults(play) {  // show results as supplied in play
    if(!mute) { play.audio.play() } // added mute option.
    verdict.textContent = play.message; 
    [winner,loser,tied].forEach(el => el.style.display = "none" );
    play.element.style.display = "block"; 
}


function compare() { // checks for draw or looks up result
    if (watson === userChoice) { 
        displayResults(plays.tied);  
    } else { 
        displayResults(results[userChoice][watson]);
    }
    
}

//Added code to mute sound
var mute = false;
sound.addEventListener("click",event => {
     mute = !mute;
     sound.textContent = mute ? "🔈" : "🔊";
});

.icon {
   cursor: pointer;
}
 .resultEmo {
   font-size : 28px;
 }

<div id="buttons">
    <img class="icon" id="rock" src="https://png.icons8.com/rock/win8/64" title="Rock" width="50" height="50">
    <img class="icon" id="paper" src="https://png.icons8.com/paper-filled/ios7/50" title="Paper Filled" width="50" height="50">
    <img class="icon" id="scissors" src="https://png.icons8.com/scissors-filled/ios7/50" title="Scissors Filled" width="50" height="50">
</div><span class="icon" id="sound" title="Click to toggle audio on and off">🔊</span>


<div class="wrapper">
  <p id="output"></p>
</div>

<div style="border: 0px solid blue; display: block;">
  <p id="myChoice"></p>
  - VS -
  <p id="cpuChoice"></p>
  <hr>
  <p id="verdict"></p>
</div>

<div class="resultEmo" id="winner" style ="display:none;">😁</div>
<div class="resultEmo" id="loser" style ="display:none;">😫</div>
<div class="resultEmo" id="tied" style ="display:none;">🙈</div>

🔊WARNING the above snippet contains sound.

Answer 2

Your code looks quite nice and is easy to understand. It can be written much shorter though.

// Start All the Variables //
var userChoice;
var watson;

var saying = "You chose " + userChoice + ". Watson chose " + watson;

Oops, the last line contains a bug. The saying variable is initialized here once and for all. It doesn't magically change whenever the userChoice or the watson changes.

To change this, you should define saying as a function. When you call a function, its code is run each time. So it should look like this:

function saying() {
    return "You chose " + userChoice + ". Watson chose " + watson;
}

Continuing …

var win = " You Win!";
var lose = " You Lose";
var tie = " We Tied";

var audioWinner = new Audio('http://facebook.design/public/sounds/Success 3.mp3');
var audioLoser = new Audio('http://facebook.design/public/sounds/Error 1.mp3');
var audioTied = new Audio('http://facebook.design/public/sounds/Collapse.mp3');

var winner = document.getElementById('winner');
var loser = document.getElementById('loser');
var tied = document.getElementById('tied');
var rock = document.getElementById("rock"); // rock button el
var paper = document.getElementById("paper"); // paper button el
var scissors = document.getElementById("scissors"); // scissors button el
var myChoice = document.getElementById("myChoice"); // user choice el
var cpuChoice = document.getElementById("cpuChoice"); // watson choice el
var verdict = document.getElementById("verdict"); // verdict el

The code looks nice, but you can improve on the variable names. The first three names describe the outcome of a single game, therefore they should use the same grammatical form. I suggest win, loss, tie.

The other names are fine. Instead of the comments at the end of the lines, you could rename rock to rockButton and so on. Then the comments become redundant.

// USER'S CHOICE
// *************
rock.addEventListener("click", function(){
  userChoice = 0;
  myChoice.innerHTML = "You chose: Rock";
  checkWatson();
  compare();
});
paper.addEventListener("click", function(){
  userChoice = 1;
  myChoice.innerHTML = "You chose: Paper";
  checkWatson();
  compare();
});
scissors.addEventListener("click", function(){
  userChoice = 2;
  myChoice.innerHTML = "You chose: Scissors";
  checkWatson();
  compare();
});

This code repeats three times. The only changes are the name of the chosen item and its choice number. To make this simpler, you can define these two things in the HTML as data attributes. It looks like this:

<img class="icon"
     id="scissors"
     src="https://png.icons8.com/scissors-filled/ios7/50"
     title="Scissors Filled"
     width="50"
     height="50"
     data-choice="2"
     data-name="Scissors">

Then, the JavaScript function can be changed to this:

function onButtonClick() {
  userChoice = this.getAttribute('data-choice');
  myChoice.innerHTML = "You chose: " + this.getAttribute('data-name');
  checkWatson();
  compare();
}

And after that, assigning the events is trivial:

rock.addEventListener("click", onButtonClick);
paper.addEventListener("click", onButtonClick);
scissors.addEventListener("click", onButtonClick);

That's all. All the data that is needed for distinguishing the buttons is fetched in the onButtonClick function.

// WATSON'S CHOICE
// ***************
function checkWatson() {
  // generates a random number between 0-2 
  randomNum = Math.floor(Math.random() * 3);

  // generate a random number and assign it to one of the 3 choices
  if (randomNum === 0) { 
    watson = "rock";
  } else if (randomNum === 1) { 
    watson = "paper";
  } else { 
    watson = "scissors";
  }
  console.log('Watson chose: ' + watson);  
}

Your code can be made a lot simpler if you keep the choices as numbers internally, instead of making them strings. There should be a single function that converts a number into a string. It can be written like this:

function choiceToName(choice) {
    return ["Rock", "Paper", "Scissors"][choice];
}

By calling console.log('Watson chose: ', choiceToName(randomNum)) you save one variable and several lines of code.

// 3 OUTCOME FUNCTIONS
// *******************
function resultsTie() {
  audioTied.play();
  verdict.innerHTML = tie; // tie
  winner.style.display = 'none';
  loser.style.display = 'none';
  tied.style.display = 'block';
}
function resultsWinner() {
  audioWinner.play();
  verdict.innerHTML = win; // win
  winner.style.display = 'block';
  loser.style.display = 'none';
  tied.style.display = 'none';
}
function resultsLoser() {
  audioLoser.play();
  verdict.innerHTML = lose;
  winner.style.display = 'none';
  loser.style.display = 'block';
  tied.style.display = 'none';
}

Again, these functions look very similar. They can be replaced with a single function that displays the outcome:

function displayOutcome() {
    var outcome = (3 + userChoice - cpuChoice) % 3;
    // 0 = tie, 1 = loss, 2 = win

    var audio = [audioTied, audioLoser, audioWinner][outcome];
    audio.play();

    var element = [tied, loser, winner][outcome];
    verdict.innerHTML = element;

    winner .style.display = 'hide';
    loser  .style.display = 'hide';
    tied   .style.display = 'hide';
    element.style.display = 'block';
}

Continuing with your code:

// COMPARE USER VS WATSON
// **********************
function compare() {
  // user chooses rock
  if (userChoice ===  randomNum) { 
    resultsTie();
  } else if (userChoice === 0 && randomNum === 1) { 
    resultsLoser();
  } else if (userChoice === 0 && randomNum === 2) { 
    resultsWinner();
  }

  // user chooses paper  
  if (userChoice === 1 && randomNum === 0) { 
    resultsWinner();
  }
  else if (userChoice === 1 && randomNum === 2) { 
    resultsLoser();
  }

  // user chooses scissors
  if (userChoice === 2 && randomNum === 0) { 
    resultsLoser();
  } else if (userChoice === 2 && randomNum === 1) { 
    resultsWinner();
  }

  cpuChoice.innerHTML = "Watson chose: " + watson;
}

I have already reduced the decision of the winner into a single line of code in the first part of the displayOutcome. Therefore the whole compare function is not necessary anymore.