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

Change random sort to use Fisher-Yates shuffle #424

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d8uv
Copy link

@d8uv d8uv commented Jan 28, 2014

Sorting by Math.random() doesn't give you a fast or a consistent shuffle. With that said, this shuffle is still bigger and more complicated than the naive approach, and I haven't done any formal benchmarking or testing. I leave it completely to you.

@albell
Copy link
Contributor

albell commented Jan 30, 2014

+1. Math.random()- 0.5 doesn't give a remotely even distribution of probabilities. I've always had a superstition about this from manual testing. Digging into it a little, it's actually true! Check out:

http://phrogz.net/JS/JavaScript_Random_Array_Sort.html

Interestingly, different browsers have very different biases, but in all browsers the weightings are dramatically unequal.

A performance-optimized modified Fisher-Yates shuffle algorithm that uses a for loop instead of a while is here:

http://stackoverflow.com/a/4206715/776121

@robertocr
Copy link

+1

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

Successfully merging this pull request may close these issues.

3 participants