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

Question

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.

@epascarello More specifically, the values returned from the `value`-properties are Strings. They can easily be converted to numbers by calling `Number(input.value)`. — Oskar Grosser, May 05 '21 at 19:35
@phantom getting the `.value` of an input always returns a string. — VLAZ, May 05 '21 at 19:36
FYI, you can cache `document.getElementById("min").value` into a variable — anotherOne, May 05 '21 at 19:37
I used ```Number(input.value)``` to turn them into numbers, still did not work. — phantom, May 05 '21 at 19:37
`document.getElementById("max").value != NaN` see [How do you check that a number is NaN in JavaScript?](https://stackoverflow.com/q/2652319) — VLAZ, May 05 '21 at 19:37
@VLAZ thank you for the info, however, it still did not fix my issue. — phantom, May 05 '21 at 19:39

obscure · Accepted Answer · 2021-05-05T20:05:47.040

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>

Thank you this fixed my issue. – phantom May 06 '21 at 20:41 — phantom, May 06 '21 at 20:41

DCR · Answer 2 · 2021-05-05T20:17:01.673

use parseInt

let min = 0;
let max = 100;

window.onload = function()
{
   
    document.getElementById("roll").addEventListener("click",function()
    {
       
      
        if(!isNaN(document.getElementById("min").value) && document.getElementById("min").value != "" &&  document.getElementById("min").value != undefined)
        {
            min = parseInt(document.getElementById("min").value);
             //console.log(document.getElementById("min").value);
        }
        if(!isNaN(document.getElementById("max").value) && document.getElementById("max").value != "" && document.getElementById("max").value != undefined)
        {
            max = parseInt(document.getElementById("max").value) + 1;
        }
        document.getElementById("output").innerHTML = `Your roll is...${random(min,max)}!`;
    });
}

 function random(min,max)
    {
        
        return Math.floor(Math.random() * (max - min) + min);
    }

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

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

2 Answers2