Why does my Button with a script not work sometimes?

Question

I've made a simple script using Javascript which generates a random number between 0-5+1 (Dice.) My issue is that when I press the button, it does nothing half the time forcing me to spam the button multiple times before a dice roll would appear.

I've looked everywhere, but couldn't find any resources as to what the issue might be. Here is my code:

<html>
<head>
</head>
<body>
<button type="button" onclick="spin()">Click here to roll the Dice!</button>
<img src="" id="demo">
<script>
function spin() {
        if (Dice() == 1) {
            document.getElementById("demo").src="http://i.imgur.com/QRTs9Ax.png";
        }
        else if (Dice() == 2) {
            document.getElementById("demo").src="http://i.imgur.com/OMz1o8U.png";
        }
        else if (Dice() == 3) {
            document.getElementById("demo").src="http://i.imgur.com/J4Xx2yO.png";
        }
        else if (Dice() == 4) {
            document.getElementById("demo").src="http://i.imgur.com/CJb2ojk.png";
        }
        else if (Dice() == 5) {
            document.getElementById("demo").src="http://i.imgur.com/8W6UL5O.png";
        }
        else if (Dice() == 6) {
            document.getElementById("demo").src="http://i.imgur.com/NGxBete.png";
        }
}

</script>
<script>
function Dice() {
    return Math.floor(Math.random() * 6) + 1;
}
</script>
</body>
</html>

paolobasso · Answer 1 · 2016-10-30T22:17:14.930

~~Because Math.floor(Math.random() * 6) + 1; could return 7.~~

EDIT

Math.floor(Math.random() * 6) + 1; can't return 7 according to Mozilla Developers because Math.random() doesn't include 1 and Math.floor just cut the value so you'll always recive a value between 1 and 6.

The proble is that with if (Dice() == x) you start the function so it will retur a diferent vaule for all the is statement. You have to build a switch like this:

function spin() {
  switch (Dice()) {
    case 1:
      document.getElementById("demo").src = "http://i.imgur.com/QRTs9Ax.png";
      break;
    case 2:
      document.getElementById("demo").src = "http://i.imgur.com/OMz1o8U.png";
      break;
    case 3:
      document.getElementById("demo").src = "http://i.imgur.com/J4Xx2yO.png";
      break;
    case 4:
      document.getElementById("demo").src = "http://i.imgur.com/CJb2ojk.png";
      break;
    case 5:
      document.getElementById("demo").src = "http://i.imgur.com/8W6UL5O.png";
      break;
    case 6:
      document.getElementById("demo").src = "http://i.imgur.com/NGxBete.png";
      break;
  }

I don't think this can return 7. I am positive it can return values between 1 to 6, see: http://stackoverflow.com/a/4960020/1333836 — Kaloyan Kosev, Oct 30 '16 at 21:54

mseifert · Accepted Answer · 2016-10-30T22:01:04.547

Store the value of dice() once to a variable and check it with the if statements. You are regenerating the the value each time so there is no guarantee you will get the current value when checking.

~~Also, as mentioned by paolo, Math.floor(Math.random() * 6) + 1; could return 7.~~

<html>
<head>
</head>
<body>
<button type="button" onclick="spin()">Click here to roll the Dice!</button>
<img src="" id="demo">
<script>
function spin() {
    var dice = Dice();
        if (dice == 1) {
            document.getElementById("demo").src="http://i.imgur.com/QRTs9Ax.png";
        }
        else if (dice == 2) {
            document.getElementById("demo").src="http://i.imgur.com/OMz1o8U.png";
        }
        else if (dice == 3) {
            document.getElementById("demo").src="http://i.imgur.com/J4Xx2yO.png";
        }
        else if (dice == 4) {
            document.getElementById("demo").src="http://i.imgur.com/CJb2ojk.png";
        }
        else if (dice == 5) {
            document.getElementById("demo").src="http://i.imgur.com/8W6UL5O.png";
        }
        else if (dice == 6) {
            document.getElementById("demo").src="http://i.imgur.com/NGxBete.png";
        }
}

</script>
<script>
function Dice() {
    return Math.floor(Math.random() * 6) + 1;
}
</script>
</body>
</html>

Soviut · Answer 3 · 2016-10-30T21:59:13.330

1

You're trying to keep your random number from returning 0 by incrementing it by + 1 after flooring. However, when Math.random() returns exactly 1.0, your multiplication will result in exactly 6, which gets floored to 6 and then gets + 1 added to it, making it 7.

Instead, only multiple your random number by 5.

return Math.floor(Math.random() * 5) + 1;

Or, even more simply, just use Math.ceil(). Which will always round up to the nearest whole number.

return Math.ceil(Math.random() * 6);

edited Oct 30 '16 at 21:59

answered Oct 30 '16 at 21:53

Soviut

88,194
49
192
260

I don't think this can return 7. I am positive it can return values between 1 to 6, see: http://stackoverflow.com/a/4960020/1333836 `Math.random()` doesn't include 1. – Kaloyan Kosev Oct 30 '16 at 21:59
@KaloyanKosev if that's the case, why isn't the function working all the time? Clearly, either a high bound or low bound value is getting generated. – Soviut Oct 30 '16 at 22:01
My guess it that the function is invoked on every if statement. So a total of 6 times. During each of this calls, it returns different value. It might simply not fulfill the condition on any given step. – Kaloyan Kosev Oct 30 '16 at 22:04

score 0 · Answer 4 · answered Oct 30 '16 at 21:57

You're calling the Dice() function on every else if statement.

I've created a fiddle where you can see on the console, which number is been generated, but your spin function would go like this:

function spin() {
  var number = Dice();
  console.log(number);
  switch (number) {
    case 1:
      document.getElementById("demo").src="http://i.imgur.com/QRTs9Ax.png";
      break;
    case 2:
      document.getElementById("demo").src="http://i.imgur.com/OMz1o8U.png";
      break;
    case 3:
      document.getElementById("demo").src="http://i.imgur.com/J4Xx2yO.png";
      break;
    case 4:
      document.getElementById("demo").src="http://i.imgur.com/CJb2ojk.png";
      break;
    case 5:
      document.getElementById("demo").src="http://i.imgur.com/8W6UL5O.png";
      break;
    case 6:
      document.getElementById("demo").src="http://i.imgur.com/NGxBete.png";
      break;
  }
}

Also you may get the same number multiple times, since it's random.

Mamdouh Saeed · Answer 5 · 2016-10-30T22:32:25.863

Just because Dice() hasn't static value and may return different value with each if statement, also may return the same random number with multiple if.

So store generated number in a variable and do your if statements as you like.

This below example based on selecting random image from imgs array and should do the task as your code do.

//add images what ever.
var imgs = [
            "http://i.imgur.com/QRTs9Ax.png",
            "http://i.imgur.com/OMz1o8U.png",
            "http://i.imgur.com/J4Xx2yO.png",
            "http://i.imgur.com/CJb2ojk.png",
            "http://i.imgur.com/8W6UL5O.png",
            "http://i.imgur.com/NGxBete.png"
            ];
    function spin() {
        var d = Dice(); //store the number in a variable
        //Dice(5); ===> if you want the max random number will be 5
        var i = d-1; //to get the right position in array
        if(imgs[i])document.getElementById("demo").src = imgs[i];
        //Also if you want additional statements use if(d == 1){}else if(d == 2){}
    }

    function Dice(max){
    max = max || imgs.length;
    return Math.floor(Math.random() * max) + 1;
    }

Why does my Button with a script not work sometimes?

5 Answers5