zeh/prando

duplicated number in range

Closed this issue · 2 comments

hey, very useful project. but i found a problem with duplicated number in range.

example:

const random = new Prando('duoreading is nice :)')
for (let i = 0; i <= 1513; i++) {
  console.log(random.nextInt(100000, 999999))
}

681267 is duplicated.

also i generated userId this way and i lost a lot of users haha. going to add a check for duplicated id now.

zeh commented

Hi @reepush,

Sorry about losing users. :(

The problem in that case is that it's constraining the range of valid numbers to 899,999 integers. Even though the algorithm can generate 536,870,912 unique integers, constraining it like that will always create collisions, sooner or later. In fact, if you ran through the entire set of valid random values, any number would end up with about 596 collisions.

This is by design; the library does not attempt to generate unique lists, but rather to get a mostly random number. In this case, what I mentioned on the documentation for nextArrayItem() applies:

Note: keep in mind that while the returned item will be random enough, picking one item from the array at a time does not guarantee nor imply that a sequence of random non-repeating items will be picked.

In your case, you were selecting an int, but it's coming from a limited set, so it's a similar problem...

Checking for a duplicated it might work, but IMO it's a kind of a work around rather than actually solving the problem. It might make your code slow too.

I think better strategies in your case for the loop would be:

  1. weak: use the sequential index itself.
  2. not as weak: use the sequential index itself, but XOR it with another number so it looks pseudo-random.
  3. strong: create an array of all valid ids (100,000...999,999), shuffle the array elements, and then read them in sequence.

Again, sorry for losing users.

@zeh Thanks for your reply! I was thinking about generating valid ids in advance, but solution with checking duplicates works good enough for me. Would be nice to have unique ids out of the box as I don't see any other solutions for this problem.