Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

characters are not generated uniformly #15

Closed
mbican opened this issue Jul 23, 2015 · 5 comments
Closed

characters are not generated uniformly #15

mbican opened this issue Jul 23, 2015 · 5 comments

Comments

@mbican
Copy link

mbican commented Jul 23, 2015

numbers 0-7 are generated more often then other characters. Because the number of characters is 62. The random number from generator is 0-255. 256 is not multiple of 62.

You should replace

var index = bf.readUInt8(i) % chars.length;

with something like this:

while((random = bf.readUInt8(i++))>=248); // 248 = 4*62
index = random % 62;

to throw away the nonuniform remainder

@eliaskg
Copy link
Contributor

eliaskg commented Sep 6, 2015

I would totally merge this if you had a working fork 😉

dchest added a commit to dchest/node-randomstring that referenced this issue Feb 12, 2016
Add tests for unique and unbiased strings.

Closes klughammer#15
dchest added a commit to dchest/node-randomstring that referenced this issue Feb 12, 2016
Add tests for unique and unbiased strings.

Closes klughammer#15
@dchest dchest mentioned this issue Feb 12, 2016
@dchest
Copy link
Contributor

dchest commented Feb 12, 2016

@mbican could you please review #18.

@weisjohn
Copy link

weisjohn commented Mar 2, 2016

👍

@eliaskg
Copy link
Contributor

eliaskg commented Mar 8, 2016

I tested this function wise and it works. Could anyone check whether characters now are really generated uniformly?

@dchest
Copy link
Contributor

dchest commented Mar 8, 2016

@eliaskg there's a test for uniformness included:

dchest@15ce2fd#diff-910eb6f57886ca16c136101fb1699231R79

If you want to see/graph the distribution, insert a few console.log's there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants