Rock, Paper, Scissors in JavaScript

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
3
down vote

favorite












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 :)





share|improve this question



























    up vote
    3
    down vote

    favorite












    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 :)





    share|improve this question























      up vote
      3
      down vote

      favorite









      up vote
      3
      down vote

      favorite











      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 :)





      share|improve this 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 :)




      // 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>





      // 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>








      share|improve this question












      share|improve this question




      share|improve this question








      edited Jan 27 at 1:27









      Martin York

      70.9k481244




      70.9k481244









      asked Jan 27 at 0:28









      user2117484

      162




      162




















          2 Answers
          2






          active

          oldest

          votes

















          up vote
          2
          down vote













          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 => 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.






          share|improve this answer























          • wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
            – user2117484
            Jan 28 at 19:33










          • Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
            – Kruga
            Jan 29 at 10:31










          • @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
            – Blindman67
            Jan 29 at 12:24

















          up vote
          1
          down vote













          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.






          share|improve this answer





















            Your Answer




            StackExchange.ifUsing("editor", function ()
            return StackExchange.using("mathjaxEditing", function ()
            StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
            StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
            );
            );
            , "mathjax-editing");

            StackExchange.ifUsing("editor", function ()
            StackExchange.using("externalEditor", function ()
            StackExchange.using("snippets", function ()
            StackExchange.snippets.init();
            );
            );
            , "code-snippets");

            StackExchange.ready(function()
            var channelOptions =
            tags: "".split(" "),
            id: "196"
            ;
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function()
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled)
            StackExchange.using("snippets", function()
            createEditor();
            );

            else
            createEditor();

            );

            function createEditor()
            StackExchange.prepareEditor(
            heartbeatType: 'answer',
            convertImagesToLinks: false,
            noModals: false,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: null,
            bindNavPrevention: true,
            postfix: "",
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            );



            );








             

            draft saved


            draft discarded


















            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186103%2frock-paper-scissors-in-javascript%23new-answer', 'question_page');

            );

            Post as a guest






























            2 Answers
            2






            active

            oldest

            votes








            2 Answers
            2






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            2
            down vote













            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 => 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.






            share|improve this answer























            • wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
              – user2117484
              Jan 28 at 19:33










            • Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
              – Kruga
              Jan 29 at 10:31










            • @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
              – Blindman67
              Jan 29 at 12:24














            up vote
            2
            down vote













            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 => 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.






            share|improve this answer























            • wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
              – user2117484
              Jan 28 at 19:33










            • Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
              – Kruga
              Jan 29 at 10:31










            • @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
              – Blindman67
              Jan 29 at 12:24












            up vote
            2
            down vote










            up vote
            2
            down vote









            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 => 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.






            share|improve this answer















            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 => 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.






            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 => 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>





            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 => 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>






            share|improve this answer















            share|improve this answer



            share|improve this answer








            edited Jan 27 at 19:00


























            answered Jan 27 at 15:16









            Blindman67

            5,4001320




            5,4001320











            • wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
              – user2117484
              Jan 28 at 19:33










            • Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
              – Kruga
              Jan 29 at 10:31










            • @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
              – Blindman67
              Jan 29 at 12:24
















            • wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
              – user2117484
              Jan 28 at 19:33










            • Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
              – Kruga
              Jan 29 at 10:31










            • @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
              – Blindman67
              Jan 29 at 12:24















            wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
            – user2117484
            Jan 28 at 19:33




            wow, thank you so much for the deep explanation. Fundamentally I knew that most the code was the same and I just needed to pass 1 or 2 things through to deliver a different outcome, but was unsure how. I really appreciate the time you spent on this. Now I understand it much better. Time to dissect your solution.
            – user2117484
            Jan 28 at 19:33












            Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
            – Kruga
            Jan 29 at 10:31




            Why did you use document.querySelector("#"+play[0]) instead of document.getElementById(play[0])? getElementById is faster than querySelector. But as you said yourself, they are already stored in a global variable, so you could just use window[play[0]].
            – Kruga
            Jan 29 at 10:31












            @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
            – Blindman67
            Jan 29 at 12:24




            @Kruga Yes using window[play[0]] or document.getElementById are alternatives. There is no particular reason I used querySelector force of habit mostly.
            – Blindman67
            Jan 29 at 12:24












            up vote
            1
            down vote













            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.






            share|improve this answer

























              up vote
              1
              down vote













              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.






              share|improve this answer























                up vote
                1
                down vote










                up vote
                1
                down vote









                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.






                share|improve this answer













                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.







                share|improve this answer













                share|improve this answer



                share|improve this answer











                answered Jan 27 at 15:21









                Roland Illig

                10.4k11543




                10.4k11543






















                     

                    draft saved


                    draft discarded


























                     


                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function ()
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186103%2frock-paper-scissors-in-javascript%23new-answer', 'question_page');

                    );

                    Post as a guest













































































                    Popular posts from this blog

                    Chat program with C++ and SFML

                    Function to Return a JSON Like Objects Using VBA Collections and Arrays

                    Will my employers contract hold up in court?