2

the JSFiddle version wasn't working so I've added a codepen instead Working Codepen

I'm fairly new to JavaScript and jQuery so I apologize if my wording isn't quite right or if the answer is extremely obvious, I'm here to learn :) So I've built a rock paper scissors game, everything works great apart from my 'New Game' button. I can't figure out how to refresh the variable 'computerChoice'. I've been able to set it so that the page will refresh, but that's not what I want to achieve. I want it so that when you click 'New Game' the variable 'computerChoice' will pick a new option depending on the random number, like when you refresh the page. I've tried setting it to null when you click 'New Game' but then that just returns numbers when you go to play the game.

HTML:

<h1>Rock Paper Scissors</h1>
<h2>Pick your weapon!</h2>
<p><button id="refresh">New Game</button></p>
<button onClick='choose("rock")' class="choices">Rock</button>
<button onClick='choose("paper")' class="choices">Paper</button>
<button onClick='choose("scissors")' class="choices">Scissors</button>
<p><button onClick='compare(user, computerChoice)' class="compares">1...2...3...</button></p>
<p><b id="you">...</b> Vs. 
<b id="computer">...</b></p>
<p><b id="results"></b></p>

JavaScript:

//user choices
var user;
var choose = function(choice) {
user = choice;
//document.getElementById("you").innerHTML = user;
}

//computer choices
var computerChoice = Math.random();
if (computerChoice < 0.34) {
  computerChoice = "rock";
} 
else if (computerChoice < 0.67) {
  computerChoice = "paper";
} 
else {
  computerChoice = "scissors";
}

//compare user choice to computer choice
function compare(choice1, choice2) {
if (choice1 === choice2) {
  document.getElementById('results').innerHTML = "How boring, you tied.";
}  
else if (choice1 === "rock") {
  if (choice2 === "scissors") {
    document.getElementById('results').innerHTML = "They'll feel that in the morning.";
} else {
  document.getElementById('results').innerHTML = "Can you breathe? I think not.";
}
}  
else if (choice1 === "scissors") {
if (choice2 === "paper") {
  document.getElementById('results').innerHTML = "Snippitysnip, you sliced them in two.";
} else {
  document.getElementById('results').innerHTML = "Ouch, looking a little blunt.";
}
} 
else if (choice1 === "paper") {
if (choice2 === "rock") {
  document.getElementById('results').innerHTML = "You smothered them, eesh!"
} else {
  document.getElementById('results').innerHTML = "You're looking a bit like a banana split."
}
} 
else {
document.getElementById('results').innerHTML = "Something's not quite right...what did you do?!"
}
}

// refresh, clear and display computer choices and results. Disable, enable buttons.
function disableButtons() {
  $('button.choices').attr('disabled', true);
}

function enableButtons() {
  $('button.choices').attr('disabled', false);
}

$('.choices').click(function() {
  $('#you').html(user); // this works on codepen but not on jsfiddle? D:
  $('#computer').html('...');
  disableButtons();
});
$('#refresh').click(function() {
  $('#results').html('');
  $('#you, #computer').html('...');
  enableButtons();
  refreshComputer();
});
$('.compares').click(function() {
  $('#computer').html(computerChoice);
});

the JSFiddle version wasn't working so I've added a codepen instead Working Codepen

7
  • Where is refreshComputer defined? Commented Oct 17, 2016 at 20:19
  • 2
    Check your console. There are errors all over the place, and most or all of them have to do with variable or function scope. Commented Oct 17, 2016 at 20:20
  • For a more complex questions like this, it'd probably be best to throw it into JSFiddle or CodePen, get it working there, then it becomes a lot easier to figure out a solution that helps. Commented Oct 17, 2016 at 20:22
  • hmm I see, however I don't get this problem with codepen. refreshComputer was meant to be removed, it was from a previous attempt Commented Oct 17, 2016 at 20:27
  • codepen.io/anon/pen/LRgPOq Commented Oct 17, 2016 at 20:32

3 Answers 3

0

Your code looks good, but it seems you tried mixing vanilla javascript with JQuery, and added some (unnecessary) handlers inside your html, probably as a part of testing out your different options.

I just refactored your code a bit, so that it would refresh the computers choice on each click on the '1..2..3' button, and that it uses JQuery for all the functions where you seem to be using it anyhow already (like setting html through JQuery or with innerHTML option, which is your own preference ofcourse, just it's nice to have 1 codebase where all code uses a similar way of doing things.

Btw, codepen or jsfiddle is nice, but this website has a perfectly valid snippet editor where you can also show how it looks like afterwards ;)

$(function() {
  // click function for choices
  $('.choices').on('click', function(e) {
    var choice = $(e.target).text();
    $('#you').html(choice);
    disableButtons();
  });
  
  // click function for refresh
  $('#refresh').click(function() {
    $('#results').html('');
    $('#you, #computer').html('...');
    enableButtons();
  });
  
  // click function for 1..2..3
  $('#compares').click(function() {
    var computerChoice = getComputerChoice(), user = getUserChoice();
    $('#computer').html(computerChoice);
    compare( user.toLowerCase(), computerChoice.toLowerCase() );
  });
  
  // gets the previously stored userChoice
  function getUserChoice() {
    return $('#you').text();
  }

  // gets a generated computer choice
  function getComputerChoice() {
    var computerChoice = Math.random();
    if (computerChoice < 0.34) {
      computerChoice = "rock";
    } else if (computerChoice < 0.67) {
      computerChoice = "paper";
    } else {
      computerChoice = "scissors";
    }
    return computerChoice;
  }

  //compare user choice to computer choice
  function compare( choice1, choice2 ) {
    var result;
    if (choice1 === choice2) {
      result = "How boring, you tied.";
    } else if (choice1 === "rock") {
      if (choice2 === "scissors") {
        result = "They'll feel that in the morning.";
      } else {
        result = "Can you breathe? I think not.";
      }
    } else if (choice1 === "scissors") {
      if (choice2 === "paper") {
        result = "Snippitysnip, you sliced them in two.";
      } else {
        result = "Ouch, looking a little blunt.";
      }
    } else if (choice1 === "paper") {
      if (choice2 === "rock") {
        result = "You smothered them, eesh!"
      } else {
        result = "You're looking a bit like a banana split."
      }
    } else {
      result = "Something's not quite right...what did you do?!"
    }
    $('#results').html(result);
  }

  // refresh, clear and display computer choices and results. Disable, enable buttons.
  function disableButtons() {
    $('button.choices').attr('disabled', true);
  }

  function enableButtons() {
    $('button.choices').attr('disabled', false);
  }
});
body {
  text-align: center;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<h1>Rock Paper Scissors</h1>
<h2>Pick your weapon!</h2>
<p>
  <button id="refresh">New Game</button>
</p>
<button class="choices">Rock</button>
<button class="choices">Paper</button>
<button class="choices">Scissors</button>
<p>
  <button id="compares">1...2...3...</button>
</p>
<p><b id="you">...</b> Vs.
  <b id="computer">...</b>
</p>
<p><b id="results"></b>
</p>

Sign up to request clarification or add additional context in comments.

5 Comments

Thank you so much! I need to have a sit down later to make sense of all this :D
could you perhaps explain this line for me, var computerChoice = getComputerChoice(), user = getUserChoice();
@lmutton Yeah sure, it declares 2 variables that get their values from the respective methods. getComputerChoice() gets each time a fresh generated value, getUserChoice() gets the text that was previously set by clicking the buttons from the html. These two values are then sent as a parameter to the compare function. Is there another part of that statement you want to have explained?
Thank you, though as much :D So ,user = getUserChoice(); could be written on a new line as var user = getUserChoice();? But the comma does that for me?
Yeah, exactly, you can declare more variables at once, and to do that you just split the declarations with a comma :)
0

Why don't you create a function of newgame and give the value of computerChoice in there. Declare variable before function

var computerChoice = 0

function newgame(){
computerChoice = Math.Random()
}

And call this function on button click on NewGame.

<button onclick="newgame()">New Game</button>

Let me know if you want something else and I will update my answer

2 Comments

This doesn't seem to work, I just get 0 back instead of rock paper or scissors.
@lmutton I updated my answer a little, can you also debug the code to check where exactly you are getting 0 after running with updated code.
0

First of all, to answer your question:

You can "refresh" (change the value) of a variable by converting it to a computed variable (or in other words, make a function that every time that it's executed, generates a new random number and returns it.) Look at my code below, and the computerChoice function.

Continuing into more detail: you are doing (at least) two things wrongly:

  1. mixing jQuery's click method with the native (inline) onClick event handler.
  2. using inline click handlers
  3. Mixing getElementByID with jQuery selection of elements

In general, adding a big library in your project and not using its methods and tools makes no sense.

If you need jQuery in your project (probably not for programs as simple as that), better go with the .on() method which gives you a bit more flexibility to do something called event delegation.

You should remove your inline click handlers, that's a certainty:

<button class="js-choices">Rock</button>
<button class="js-choices">Paper</button>
<button class="js-choices">Scissors</button>

then attach your event handler on the js-choices element, using event delegation:

$('body').on('click', '.js-choices', function() {
  choose($(this).text());
});

function choose(choice) {
  user = choice;
}

I went through your code and made it work. I saw the following additional problems:

  1. you use too many ifs: that almost always is a good candidate for switch statements
  2. you had logical holes in your conditions (when comparing for the value of Math.random())

The snipped below is not perfect, but it's definitely an improvement:

var user;

$('body').on('click', '.js-choices', function() {
  user = $(this).text();
  $('#you').html(user);
  $('#computer').html('...');
  disableButtons();
});


//computer choices
function computerChoice() {
  var temp = Math.random();
  if (temp < 0.34) {
    return "rock";
  }
  if (temp >= 0.34 && temp < 0.67) {
    return "paper";
  } else {
    return "scissors";
  }
}

//compare user choice to computer choice

// cache your results element to only access it 1 time and improve performance
// the naming convention says, use a variable with a '$' in front if its
// a jquery object

var $results = $('#results');

function compare(user, computer) {
  if (user === computer) {
    $results.text("How boring, you tied.");
  } else if (user === "rock") {
    if (computer === "scissors") {
      $results.text("They'll feel that in the morning.");
    } else {
      $results.text("Can you breathe? I think not.");
    }
  } else if (user === "scissors") {
    if (computer === "paper") {
      $results.text("Snippitysnip, you sliced them in two.");
    } else {
      $results.text("Ouch, looking a little blunt.");
    }
  } else if (user === "paper") {
    if (computer === "rock") {
      $results.text("You smothered them, eesh!");
    } else {
      $results.text("You're looking a bit like a banana split.");
    }
  } else {
    $results.text("Something's not quite right...what did you do?!");
  }
}

// refresh, clear and display computer choices and results. Disable, enable buttons.
function disableButtons() {
  $('.js-choices').attr('disabled', true);
}

function enableButtons() {
  $('.js-choices').attr('disabled', false);
}

$('#refresh').click(function() {
  $('#results').html('');
  $('#you, #computer').html('...');
  computerChoice();
  enableButtons();
});
$('.js-play').click(function() {
  compare(user, computerChoice());
  $('#computer').html(computerChoice);
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
<h1>Rock Paper Scissors</h1>
<h2>Pick your weapon!</h2>
<p>
  <button id="refresh">New Game</button>
</p>
<button class="js-choices">rock</button>
<button class="js-choices">paper</button>
<button class="js-choices">scissors</button>
<p>
  <button class="js-play">1...2...3...</button>
</p>
<p><b id="you">...</b> Vs.
  <b id="computer">...</b></p>
<p><b id="results"></b></p>

8 Comments

I would agree with most of your refactoring, but you call the compare function without arguments which ties the game at each attempt (though I think that was an oversight on your part ;)).
indeed :) I was just correcting stuff. The compare() function in general is a mess...Those nested ifs..
It ain't bad for a first attempt, block scopes, steady use of semicolon, ofcourse it can be improved, but in all honesty, I do remember and have some old code laying around where I am also think, ouch ;) Btw wouldn't it benefit from setting the result once after all the ifs? :) Repeating a statement so often, lends to excluding it and setting only once :)
I have a strong feeling most of this was copy paste.. especially the mixing of native/jQuery, the inline event handlers.. Proof that front-end is been taught in a chaotic way :(
Thank you so much for the response. I didnt copy and paste. Ive only been learning JavaScript casually for the last month or so. After completing this game on codecamp, I decided to implement what I learned re'write it out
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.