Random numbers generator: function arguments appear as undefined

Question

I am working on a random numbers generator. It is basically a function with 3 parameters (arguments):

1. count, how many numbers will be generated
2. min, the minimum value of the generated numbers
3. max, the maximum value of the generated numbers

Here is the code:

function numGenerator(count, min, max) {
  var arr = [],
  count = document.getElementById('counter').value,
  min = document.getElementById('min_number').value,
  max = document.getElementById('max_number').value;
  while (arr.length < count) {
    var randomnumber = Math.floor(Math.random() * max) + min;
    if (arr.indexOf(randomnumber) > -1) continue;
    arr[arr.length] = randomnumber;
  }
  // Wrapp each number in a div and put them
  // all inside a container
  for (var i = 0; i < arr.length; i++) {
    document.getElementById('numbers_container').innerHTML += '<div class="number">' + arr[i] + '</div>';
  }
}
document.getElementById('generateBtn').addEventListener("click", numGenerator(count, min, max));

<div class="container">
  <div id="numbers_container"></div>
  <div id="conditions">
    <span>Generate</span>
    <input type="text" id="counter"> <span>numbers, between</span>
    <input type="text" id="min_number"> <span>and</span>
    <input type="text" id="max_number">
  </div>
  <button id="generateBtn">Generate numbers</button>
</div>

This gives the error: Uncaught ReferenceError: count is not defined. Why? Where is my mistake?

Answer 1

You code is calling the method numGenerator and passing in 3 variables to it. Those 3 variables are not defined anywhere in your code. So that is the error. There is no reason for you to be passing in those variables in the first place.

Your code should just be

document.getElementById('generateBtn').addEventListener("click", numGenerator)

Second issue is count, min, and max are strings, not numbers, so you need to convert them to numbers with either parseInt, parseFloat, Number.

Finally your logic for the random number needs some work. Check out Generating random whole numbers in JavaScript in a specific range?

Answer 2

Thanks to Matheus Cuba and others, I have managed to make a working script (or so I believe):

function numGenerator(count, min, max) {
  var arr = [],
  count = parseInt(document.getElementById('counter').value),
  min = parseInt(document.getElementById('min_number').value),
  max = parseInt(document.getElementById('max_number').value);
  while (arr.length < count) {
    var randomnumber = Math.floor(Math.random() * max) + min;
    if (arr.indexOf(randomnumber) > -1) continue;
    arr[arr.length] = randomnumber;
  }
  // Clear the numbers container before generating new numbers
  document.getElementById('numbers_container').innerHTML="";
  // Wrapp each number in a div and put them
  // all inside a container
  for (var i = 0; i < arr.length; i++) {
    document.getElementById('numbers_container').innerHTML += '<div class="number">' + arr[i] + '</div>';
  }
}
document.getElementById('generateBtn').addEventListener("click", numGenerator);

<div class="container">
  <div id="numbers_container"></div>
  <div id="conditions">
    <span>Generate</span>
    <input type="text" id="counter"> <span>numbers, between</span>
    <input type="text" id="min_number"> <span>and</span>
    <input type="text" id="max_number">
  </div>
  <button id="generateBtn">Generate numbers</button>
</div>

Answer 3

If you want to set count, min and max to the inputs selected by the user on click, you will have to do that outside the function. It doesn't make sense to redefine variables you have used as arguments inside the function. I would try first removing these lines from the function:

 count = document.getElementById('counter').value,
 min = document.getElementById('min_number').value,
 max = document.getElementById('max_number').value;

 (note: those should also be semicolons and not commas at the end)

then make the onclick event look more like this:

document.getElementById('generateBtn').onclick = function() {
  var newCount = parseInt(document.getElementById('counter').value);
  var newMin = parseInt(document.getElementById('min_number').value);
  var newMax = parseInt(document.getElementById('max_number').value);
  numGenerator(newCount, newMin, newMax);
};

That's what I would try doing anyway. Hope it helps!

Answer 4

A version like this solves the various problems that have been seen in other answers. It's not code I would be proud of... but it's a start.

function numGenerator() {
  let count = Number(document.getElementById('counter').value);
  let min = Number(document.getElementById('min_number').value);
  let max = Number(document.getElementById('max_number').value);
  // ensure min is no bigger than max
  [min, max] = [Math.min(min, max), Math.max(min, max)]
  // ensure count is not higher than the number of values available
  count = Math.min(count, max - min + 1)
  let arr = []; 
  while (arr.length < count) {
    // properly find a random value in [min, max]
    let randomnumber = Math.floor(Math.random() * (max - min + 1)) + min;
    if (arr.indexOf(randomnumber) > -1) continue;
    arr.push(randomnumber);
  }
  // Clear the numbers container before generating new numbers
  document.getElementById('numbers_container').innerHTML="";
  // Wrapp each number in a div and put them
  // all inside a container
  for (var i = 0; i < arr.length; i++) {
    document.getElementById('numbers_container').innerHTML += '<div class="number">' + arr[i] + '</div>';
  }
}

document.getElementById('generateBtn').addEventListener("click", numGenerator)

<div class="container">
  <div id="numbers_container"></div>
  <div id="conditions">
    <span>Generate</span>
    <input type="text" id="counter"> <span>numbers, between</span>
    <input type="text" id="min_number"> <span>and</span>
    <input type="text" id="max_number">
  </div>
  <button id="generateBtn">Generate numbers</button>
</div>

This version does not deal with possible non-integer input, and it makes some assumptions about what you would prefer to do if min is larger than max, or if count is larger than the number of potential values.

It's also poorly factored, mixing concerns of number generation with DOM input and DOM output.

Changed to fix these are left as an exercise for the reader. :-)