Skip to content

randomizing number function with two parameters is not randomizing between the parameters

I have made a function that is suppose too randomize a number after being givin two numbers to work with: min and max. I call this function: random(min,max). I also have a button to call this function, but first detects to see if two inputs are not undefined, NaN, or “”. My program seemed to work at first but the very moment I used 5 and 10, 5 being my min and 10 being my max, it started to just use the default numbers I set for it, 0 and 100.

javascript

window.onload = function()
{
    function random(min,max)
    {
        return Math.floor(Math.random() * (max - min) + min);
    }
    document.getElementById("roll").addEventListener("click",function()
    {
        console.log(document.getElementById("min").value);
        let min = 0;
        let max = 100;
        if(!isNaN(document.getElementById("min").value) && document.getElementById("min").value != "" &&  document.getElementById("min").value != undefined)
        {
            Number(min = document.getElementById("min").value);
        }
        if(!isNaN(document.getElementById("max").value) && document.getElementById("max").value != "" && document.getElementById("max").value != undefined)
        {
            Number(max = document.getElementById("max").value + 1);
        }
        document.getElementById("output").innerHTML = `Your roll is...${random(min,max)}!`;
    });
}

html

<center>
            <form>
                <label>min number</label><br>
                <input type='number' placeholder=0 id='min'>
            </form>
            <form>
                <label>max number</label><br>
                <input type='number' placeholder=100 id='max'>
            </form>
            <button id='roll'>
                roll dice
            </button>
            <p id='output'>
                your roll is...
            </p>
</center>

The issue is basically that it is rolling like it should at first, without inputting a new min or max, but then when you input a new min it does not follow the rules of that min like it should. I tested with just the max, and it works fine for that for some reason.

Answer

First, you don’t need to chain all those operations:

!isNaN(document.getElementById("min").value) && document.getElementById("min").value != "" &&  document.getElementById("min").value != undefined

In fact, it’s sufficient to check for two of those:

if(!isNaN(document.getElementById("min").value) && document.getElementById("min").value!="")

The value returned by the <input> element is always a string and the isNaN function tries to convert anything that’s passed in as a parameter to a Number, well except for an empty string.

The next mistake is this:

Number(min = document.getElementById("min").value);

Actually this doesn’t do anything. I think you wanted to assign the variable min the value of the <input> textfield, but it can’t be done that way. It’s needs to look like this:

min = parseInt(document.getElementById("min").value);

So if we put everything together:

function random(min, max) {
  return Math.floor(Math.random() * (max - min) + min);
}
document.getElementById("roll").addEventListener("click", function() {
  let min = 0;
  let max = 100;

  if (!isNaN(document.getElementById("min").value) && document.getElementById("min").value != "") {
    min = parseInt(document.getElementById("min").value);
  }
  if (!isNaN(document.getElementById("max").value) && document.getElementById("max").value != "") {
    max = parseInt(document.getElementById("max").value) + 1;
  }

  document.getElementById("output").innerHTML = `Your roll is...${random(min,max)}!`;
});
<form>
  <label>min number</label><br>
  <input type='number' placeholder=0 id='min'>
</form>
<form>
  <label>max number</label><br>
  <input type='number' placeholder=100 id='max'>
</form>
<button id='roll'>
                roll dice
            </button>
<p id='output'>
  your roll is...
</p>