Find unique range numbers

Question

im using the following code which is working. The code provide range of random values but unique one i.e. it doesn't return the same values until the full range was provided ,what do you think and doest in ES6 there is shorter way to achieve this?

var range = [10, 20];
var not = [];

function randomRange(range, n) {
  if (not.length >= range[1] - range[0]) {
    not.length = 0;
  }
  var curr = [];
  var res = [];
  for (let i = range[0]; i < range[1]; i++) {
    if (!not.some(function(num) {
      return i == num
    }) && not.length < range[1] - range[0]) {
      curr.push(i)
    }
  }
  for (let i = 0; i < n; i++) {
    var j = curr.splice(Math.floor(Math.random() * curr.length), 1)[0];
    res[i] = not[not.length] = j;
  }

  return res.filter(Boolean)
}

function log(result) {
  document.body.innerHTML += JSON.stringify(result) + "<br>"
}

window.onload = function() {

log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
log(randomRange(range, 3));
}

Answer 1

Confusing API

The current API is a bit confusing. range is a parameter of randomRange, but in the usage example you gave you always call the function with the same range value. It's unclear if you ever intend to call it with a different value. And if you do, it's unclear what realistic purpose it could serve. The behavior would seem rather chaotic.

Also note that the current API and the global not variable prevents you from using multiple different random ranges at the same time.

Instead of making range a parameter of randomRange, it would be better to use it once for initialization, and store it in an internal data structure not accessible from outside.

An array seems a strange choice for storing two values. Instead of range as an array, why not use simply start and end variables?

Fragile implementation

The implementation is very fragile. not is quite crucial for the inner workings of the randomRange function, but it's easily accessible in the global namespace, making it vulnerable to accidental or malicious modifications.

Performance

The current implementation is inefficient. On each call, it scans the range and populates curr with values that are not in not, and then selects random values from curr. It would be better to shuffle the range of values, splice from the shuffled range, and reshuffle when it's emptied.

Alternative implementation

Consider this alternative implementation that solves all the above issues:

var shuffle = function(arr) {
  for (let i = arr.length - 1; i > 0; i--) {
    let target = nextInt(i + 1);
    swap(arr, i, target);
  }
};

var swap = function(arr, i, j) {
  var tmp = arr[i];
  arr[i] = arr[j];
  arr[j] = tmp;
};

var nextInt = function(bound) {
  if (bound === undefined) {
    bound = Number.MAX_SAFE_INTEGER;
  }
  return Math.floor(bound * Math.random());
};

var recyclingRandomRange = function(start, end) {
  var range = [];
  var resetRange = function() {
    for (let i = start; i < end; i++) {
      range.push(i);
    }
    shuffle(range);
  };

  return function(n) {
    if (range.length === 0) {
      resetRange();
    }
    return range.splice(0, n);
  };
};

This usage would be equivalent to your example:

var ranger = recyclingRandomRange(10, 20);
log(ranger(3));
log(ranger(3));
log(ranger(3));
log(ranger(3));
log(ranger(3));
log(ranger(3));
log(ranger(3));

Answer 2

If what you want is to call the same function with the same array and obtain random numbers from that array every time, but without them repeating, well, that's almost literally the definition of "shuffle".

Have a look at this answer on stackoverflow. The idea in your case is that you shuffle the array and pick the first (or last) n elements.

That means something like (almost entirely copied from linked answer):

function shuffle(array) {
  var currentIndex = array.length, temporaryValue, randomIndex;

  while (0 !== currentIndex) {
    randomIndex = Math.floor(Math.random() * currentIndex);
    currentIndex -= 1;
    temporaryValue = array[currentIndex];
    array[currentIndex] = array[randomIndex];
    array[randomIndex] = temporaryValue;
  }

  return array;
}

var arr = [];
for (var i = 10; i <= 20; i++) {
   arr.push(i);
}
shuffle(arr);
console.log(arr.slice(0,3));
console.log(arr.slice(3,6));

The advantage is that you need to randomize only once, no need to loop through the array and generate random numbers every time.

The disadvantage is that if you want different numbers, you'll have to keep track of the index by yourself, because requesting arr.slice(0,3) will always return the same numbers. But considering that you'd have to keep track of which numbers have already been used anyway, this should not be a problem.

Also, this may actually not be a disadvantage depending on what you use the function for. (Of course you could shuffle every time, but that would lose any speed benefit of shuffling).

Another approach would be to shuffle once and then having a function that returns the first n numbers but also removes them from the list. The obvious disadvantage in that case would be that you have to keep a copy of your original array, and when the shuffled array is empty, copy it and shuffle it again.

If you still want to use your approach (which works anyway), at least make sure to use meaningful variable names. In my opinion not is probably the worst possible name for a variable.