Javascript unique random number generator is not generating unique numbers

Question

I'm trying a new random number generator that is supposed to generate numbers without duplicates, but when I try to apply it to my page it produces many duplicates. At least 60% of the time there was a dupe, one time a triplicate, and two sets of duplicates.

I'm trying the answer from Generate unique random numbers between 1 and 100 , and it seems to work as is even when I limit it to 20 numbers. Ran it 40 times with zero duplicate numbers. It's when I try to put it in my existing function that it falls apart. Any idea what I'm missing here? This is a continuation of my previous question Fill table with random images

/* The part of what each image url has in common
   ⍟ var base = 'images/Image_'
   */
var base = 'images/Image_';
var suff = '.jpg';

function randomCellBG(base) {

  // Reference the <table>
  var T = document.getElementById('mainTable');

  /* Collect all .cell into a NodeList and convert
  || it into an array
  */
  var cellArray = Array.from(T.querySelectorAll('.cell'));

  // map() the array; run a function on each loop...
  cellArray.map(function(cel, idx) {

    // Get a random number 1 - 9
    var arr = []
    while (arr.length < 9) {
      var ran = Math.ceil(Math.random() * 40)
      if (arr.indexOf(ran) > -1) continue;
      arr[arr.length] = ran;
    }

    /* Concatenate base and random number to form
    || a string of a url of an image
    ⍟ result: "images/Image_08.jpg"
    */
    var img = base + ran.toString() + suff;

    /* Assign that url as the value to the 
    || backgroundImage property of the .cell
    || in current iteration
    */
    cel.innerHTML = "<img src='" + img + "'/>";

  });

}

You're generating a new set of 5 unique random numbers for each item in `cellArray`. There could be overlap between those sets. Side note: avoid using `.map` if what you actually want to do is `.forEach`. They serve different purposes. — JLRishe, Nov 20 '17 at 12:03
Sorry, forgot to change the code back after testing how it performed with a constrained set of numbers. I want to choose 9 from a set of 40. — dano, Nov 20 '17 at 12:11
you should generate the random numbers before `cellArray.map`, not inside it — Slai, Nov 20 '17 at 12:32

score 1 · Accepted Answer · answered Nov 20 '17 at 12:34

if I understand it correctly, then you need: 1. change map to forEach 2. you need to move the array where you save already generated numbers out of the forEach function 3. change the number generator loop

/* The part of what each image url has in common
⍟ var base = 'images/Image_'
*/
var base = 'images/Image_';
var suff = '.jpg';

function randomCellBG(base) {

  // Reference the <table>
  var T = document.getElementById('mainTable');

  /* Collect all .cell into a NodeList and convert
  || it into an array
  */
  var cellArray = Array.from(T.querySelectorAll('.cell'));

  // initialize the array
  var arr = []
  // map() the array; run a function on each loop...
  cellArray.forEach(function (cel, idx) {

    // generate numbers 1-40 and check if they were already generated
    do {
      var ran = Math.ceil(Math.random() * 40)
    } while (arr.indexOf(ran) > -1);
    //save the newly generated unique number
    arr[arr.length] = ran;

    /* Concatenate base and random number to form
    || a string of a url of an image
    ⍟ result: "images/Image_08.jpg"
    */
    var img = base + ran.toString() + suff;

    /* Assign that url as the value to the 
    || backgroundImage property of the .cell
    || in current iteration
    */
    cel.innerHTML = "<img src='" + img + "'/>";
  });
}

score 0 · Answer 2 · answered Nov 20 '17 at 12:33

Why don't you shuffle the array and then take the first n items which you need for the table?

You can use a shuffle algorithm and then simply slice the elements you need for your image table.

Shuffle means that there will be no duplicates derived from using a random function.

Something along these lines:

function shuffleArray(array) {
    array.forEach(function(e, i) {
        exch(array, i, Math.floor(Math.random() * (i + 1)));
    });
    function exch(array, i, j) {
        const temp = array[i];
        array[i] = array[j];
        array[j] = temp;
    }
}

var array = [0, 1, 2, 3, 4, 5, 6, 7, 8];
shuffleArray(array);
console.log(array.slice(0, 3));

array = ["a", "b", "c", "d", "e", "f"];
shuffleArray(array);
console.log(array.slice(0, 3));

score 0 · Answer 3 · answered Nov 20 '17 at 12:35

It looks like what you need to do is generate your random numbers, and then apply them to each cell:

function generateNumbers(count, upperBound) {
    if (count > upperBound) { throw new Error('count cannot be more than upper bound'); }

    var arr = []

    while (arr.length < count) {
      var ran = Math.ceil(Math.random() * upperBound)
      if (arr.indexOf(ran) > -1) continue;
      arr[arr.length] = ran;
    }

    return arr;
}

function randomCellBG(base) {

  // Reference the <table>
  var T = document.getElementById('mainTable');

  /* Collect all .cell into a NodeList and convert
  || it into an array
  */
  var cellArray = Array.from(T.querySelectorAll('.cell'));

  var nums = generateNumbers(cellArray.length, 40);

  // map() the array; run a function on each loop...
  cellArray.forEach(function(cel, idx) {
    /* Concatenate base and random number to form
    || a string of a url of an image
    ⍟ result: "images/Image_08.jpg"
    */
    var img = base + nums[idx] + suff;

    /* Assign that url as the value to the 
    || backgroundImage property of the .cell
    || in current iteration
    */
    cel.innerHTML = "<img src='" + img + "'/>";

  });

}

Javascript unique random number generator is not generating unique numbers

3 Answers3