vega / datalib

JavaScript data utility library.
http://vega.github.io/datalib/
BSD 3-Clause "New" or "Revised" License
731 stars 133 forks source link

Function for array permutation #70

Closed mcorrell closed 8 years ago

mcorrell commented 8 years ago

Permutes an array in place. Permuted with a Knuth shuffle aka the Fisher-Yates shuffle.

mcorrell commented 8 years ago

For my own code I prefer incrementing for loops over decrementing while loops. Both are O(n). The decrementing version I think will be slightly faster in practice (the space of possible swaps gets smaller and smaller with each decrement, for instance). The decrementing version is also technically more accurate, since the incrementing version allows multiple swaps of the same value (which means the identity permutation is slightly less likely, among other properties).

Switching over is certainly easy enough.

domoritz commented 8 years ago

I don't care about the performance difference but found the decrementing version to be more understandable.

domoritz commented 8 years ago

More importantly, why do you create a copy when the explicit advantage of the algorithm is that you can shuffle in place? If it is explicit (e.g. no return value), users can make a copy themselves if they need to.

mcorrell commented 8 years ago

Also personal preference: I prefer these methods to not have side effects.

But precedent both externally (for instance php's std shuffle() method) and internally (most of the other array methods in util have side effects) backs you up, so I'll switch over.

domoritz commented 8 years ago

LGTM. I changed the tests a bit but this looks good to go.