0

I am trying to choose random unique numbers everytime when I click button. For this my function is:

const chooseNumber = () => {
    var r = Math.floor(Math.random() * 75) + 1;
    console.log(r)
    while(selectedNumbers.indexOf(r) === -1) {
      selectedNumbers.push(r);
    }
    console.log(selectedNumbers);
  };

But the problem is if the random number is already on my list, I need to click the button again to generate new number and it goes until it find the number which is not on the list. But I want to generate number which is not on the list directly so I dont need to click the button everytime. Thanks for you helps.

breking bed
  • 135
  • 2
  • 14

3 Answers3

1

You are in a right track, except the while loop should be for random number generator, not pushing number into an array:

const selectedNumbers = [];
const chooseNumber = () => {
    let r;
    do
    {
      r = Math.floor(Math.random() * 75) + 1;
    }
    while(selectedNumbers.indexOf(r) > -1)
    selectedNumbers.push(r);
    console.log(r, "["+selectedNumbers+"]");
  };
<button onclick="chooseNumber()">Generate</button>

Note, that this might eventually lead to a freeze, since there is no fail safe check if array is full, so to battle that we should also check length of the array:

const selectedNumbers = [];
const maxNumber = 75;
const chooseNumber = () => {
  let r;
  do
  {
    r = ~~(Math.random() * maxNumber) + 1;
  }
  while(selectedNumbers.indexOf(r) > -1 && selectedNumbers.length < maxNumber)
  if (selectedNumbers.length < maxNumber)
    selectedNumbers.push(r);
  else
    console.log("array is full");

  console.log(r, "["+selectedNumbers+"]");
};


for(let i = 0; i < 76; i++)
{
  chooseNumber();
}
<button onclick="chooseNumber()">Generate</button>
vanowm
  • 9,466
  • 2
  • 21
  • 37
0

Don't rely on a loop to generate a unique (unseen) integer in a limited range.

First, once all of the values in the range have been exhausted there will be no possibilities left, so you'll be left in an endless loop on the next invocation.

Second, it's wasteful of the processor because you are generating useless values on each invocation.

Instead, generate all of the values in range in advance (once), then shuffle them and get the last one from the array on each invocation (and throw an error when none remain):

/**
 * Durstenfeld shuffle
 *
 * - https://stackoverflow.com/a/12646864/438273
 * - https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#The_modern_algorithm
 */
function shuffleArray (array) {
  for (let i = array.length - 1; i > 0; i--) {
    const j = Math.floor(Math.random() * (i + 1));
    [array[i], array[j]] = [array[j], array[i]];
  }
}

/** Get shuffled array of all integers in range */
function generateShuffledIntegerArray (min, max) {
  const arr = [];
  for (let i = min; i <= max; i += 1) arr.push(i);
  shuffleArray(arr);
  return arr;
}

const getUniqueInt = (() => {
  const numbers = generateShuffledIntegerArray(1, 75);

  return () => {
    const n = numbers.pop();
    if (typeof n === 'number') return n;
    throw new Error('No unique numbers remaining');
  };
})();

// Will log 75 integers, then throw on the 76th invocation:
for (let i = 1; i <= 76; i += 1) {
  const n = getUniqueInt();
  console.log(`${i}:`, n);
}

Code in TypeScript Playground

jsejcksn
  • 27,667
  • 4
  • 38
  • 62
  • This seems to be very inefficient way, consumes a lot of memory and might crash entire process if used large range (i.e. it crashes on my computer with 200000000 range during generating the array) – vanowm Jun 19 '22 at 15:07
  • @vanowm All software solutions involve time-space complexity trade-offs, and this answer was written to address the constraints of the problem in the question (the range of integers 1 to 75). And I guarantee that it will outperform the loop solution for a range of that size as the memoized set approaches the full range. – jsejcksn Jun 19 '22 at 15:14
  • It is indeed faster after 35+ generated numbers with a range of 75 and its exponentially faster the more numbers it generates, however for 10 generated numbers it's 80% slower... and if range is 750 it needs to generate 140+ numbers to be more efficient. I just wanted to point this out, since it's not mentioned in the answer. – vanowm Jun 19 '22 at 15:52
-1

Your while loop is unnecessary. You could use an "if" statement instead.

To avoid clicking again on your button, you can do a recursive function like:

const chooseNumber = () => {
    var r = Math.floor(Math.random() * 75) + 1;
    console.log(r)
    if(selectedNumbers.indexOf(r) === -1) {
      selectedNumbers.push(r);
      console.log(selectedNumbers);
    } else {
      chooseNumber();
    }
};
Nimarel
  • 317
  • 2
  • 2
  • with a large range this might lead to a `Uncaught RangeError: Maximum call stack size exceeded` – vanowm Jun 19 '22 at 14:30