Why does my JavaScript code not work on refresh?

Question

I'm taking a Udemy course and one of the exercises is to build a Dice game.

Basically it's just two dice (for each player/stakeholder) on screen both showing the value of 6. On refresh, it should 'roll' the dice (generate a random side for each dice, which are all images), and the header text should change to state the outcome of the game.

But my code doesn't work on refresh. I'm trying to find out why. Here's a comparison of my code VS the instructor's code:

This is my code, which doesn't work on refresh (but I've tested it and it works when I run it as a function).

//Generating Random Numbers for Dice 1 & Dice 2
    var RandomNo1 = (Math.floor(Math.random() * 6) + 1)
    var RandomNo2 = (Math.floor(Math.random() * 6) + 1)

//Making a var for image sources Dice 1-6 (images/dice#.png)
    var imageSource1 = ("images/dice" + RandomNo1 + ".png")
    var imageSource2 = ("images/dice" + RandomNo2 + ".png")

//set.Attribute function to change the img source for the dice
    document.querySelector('.player1').setAttribute("src", imageSource1);
    document.querySelector('.player2').setAttribute("src", imageSource2);

//Setting conditional statements to change the header depending on results of the dice roll.

     if (RandomNo1 > RandomNo2) {
         document.querySelector("h1").innerText="Player 1 Wins!";
       }

     else if (RandomNo2 > RandomNo1) {
         document.querySelector("h1").innerText="Player 2 Wins!";
       }

     else {
         document.querySelector("h1").innerText="It's a draw!";
       }

This is the instructor's code, which does work on refresh.

var randomNumber1 = Math.floor(Math.random() * 6) + 1; //1-6

var randomDiceImage = "dice" + randomNumber1 + ".png"; //dice1.png - dice6.png

var randomImageSource = "images/" + randomDiceImage; //images/dice1.png - images/dice6.png

var image1 = document.querySelectorAll("img")[0];

image1.setAttribute("src", randomImageSource);


var randomNumber2 = Math.floor(Math.random() * 6) + 1;

var randomImageSource2 = "images/dice" + randomNumber2 + ".png";

document.querySelectorAll("img")[1].setAttribute("src", randomImageSource2);


//If player 1 wins
if (randomNumber1 > randomNumber2) {
  document.querySelector("h1").innerHTML = " Play 1 Wins!";
}
else if (randomNumber2 > randomNumber1) {
  document.querySelector("h1").innerHTML = "Player 2 Wins! ";
}
else {
  document.querySelector("h1").innerHTML = "Draw!";
}

How does her page change on refresh? Why doesn't it change immediately upon loading? Why doesn't my code, like hers, work on refresh?

Answer 1

Your script is fine. In essence it does the same as the script your teacher provided. To be honest, your script is better on several aspects, not in the least because you have added explanatory comments throughout, and I really like to see someone using .innerText, and not .innerHTML. The latter is really intended to inject HTML, so you did right to opt for .innerText. Personally, I would use .textContent, but that is just a detail.

The only explanation that is left, is that your script runs too soon, when the DOM is not ready yet, and so the querySelector() calls return undefined.

Move your <script> element towards the end of your HTML document, after the other elements you need to address in your script. A good place to put it, is right before the closing </body> tag.

Answer 2

welcome to stackoverflow!

There are a few mistakes in your code:

1. Do not begin your variable names with capital lettes, these are for naming constructors. (It's some sort of unspoken law to make your code better readable for other developers as well)

2. I don't think this is a problem in your code you showed us. I think you should paste the script tag on the end of the html file, so document.querySelector() can select the html elements.

3. I made a pen here where I took your code and show you that it works. (I just changed the $element.setAttribute("src") to $element.innerText)

   • You can also use $element.src = instead of $element.setAttribute("src")