Change the shuffle to the Fisher-Yates shuffle #72

Merged
merged 1 commit into from Feb 28, 2013

Projects

None yet

2 participants

@d8uv

The current "Shuffling Array Elements" page recommends a naive algorithm, which produces a slow and biased shuffle. Included is a highly idiomatic refactorization of the Fisher-Yates shuffle (aka the Knuth Shuffle), a less-idiomatic-but-better refactorization, and a version of the algorithm that adds to Array.prototype, for those that program in that style.

@d8uv d8uv Change the shuffle to the Fisher-Yates shuffle
The current "Shuffling Array Elements" page recommends a naive algorithm, which produces 
a slow and biased shuffle. Included is a highly idiomatic refactorization of the Fisher-
Yates shuffle (aka the Knuth Shuffle), a less-idiomatic-but-better refactorization, and a
version of the algorithm that adds to Array.prototype, for those that program in that 
style.
fa13f3b
@dbrady
CoffeeScript Cookbook member

THIS. This is what a fantastic recipe entry looks like. I saw this email and thought "oh no, they've deleted the simple shuffle and replaced it with a needlessly complex one". I came in here expecting to write a message asking you to preserve the old shuffle, but then explain when and why it might be unsuitable, and propose the Fisher-Yeates shuffle as a replacement. When I read your diff I wanted to cry with joy. d8uv, this is awesome. THANK YOU.

This is the happiest "I hereby give you commit rights" message I've written to date. :-) Welcome!

@dbrady dbrady merged commit d0ee5cb into coffeescript-cookbook:master Feb 28, 2013
@dbrady
CoffeeScript Cookbook member

Actually, rereading it, might be better if it started with array.shuffle(), presented for fairly simple cases of randomness. Is it slightly biased, or is it obviously biased? I haven't run the Chi-squareq analysis on it to see how broken it is. I wonder if we could give some examples for when it works as a naive example, along with a specific use case that shows just how bad it can be--sort of like how you should never use modulus on Linear Congruential generators because rand() % 2 in the C stdlib will return 0,1,0,1,0,1,... etc.

@d8uv d8uv deleted the unknown repository branch Feb 28, 2013
@d8uv

The chi-square analysis for the random.sort vs. fisher-yates has been done by Rob Weir. I haven't done a proper formal analysis of my refactorization, so if you're gonna drag out R you might as well check to see if my math-fu is strong, but my informal tests seem to pan out wonderfully.

The reason I didn't write it in a "this is what everyone uses, these are the problems, and this is the solution" way is... I find that style to be a little complex when you're trying to learn something specific. People are here to learn how best to shuffle in Coffeescript, and Knuth Shuffle is the best way. People aren't here to learn about a bunch of different methods and their pitfalls, they just want the goods, and to get out. Adding that supplementary stuff at the end is great, and I'd be really curious as to what that'd look like, but the solution should stay at the top

And as to if the naive solution is good enough... I'd say probably never. Doing it properly only takes up 5 lines of source, is a lot faster, and is free from bias. When you want something random, you really want it to be random, not random-ish.

@dbrady
CoffeeScript Cookbook member
@d8uv d8uv added a commit that referenced this pull request Mar 1, 2013
@d8uv d8uv Make Array::shuffle safer
As mentioned by @dbrady in #72 we shouldn't overwrite a native `Array::shuffle`
0648bcf
@d8uv

I... actually disagree with that. If Array::shuffle gets into native javascript, then code I wrote in 0648bcf automatically becomes a polyfill. It'll use the native method if it's available, and will use our method if it isn't. And regardless, most of the time, we should be using a utility library like Lo-dash or Underscore, because they smooth out inconsistencies when the native methods differ from platform to platform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment