reuse lcg during pack #193
Conversation
There was a problem hiding this comment.
I works, and I'm happy with the renaming to packSiblings which clarifies things for me.
A detail is, I don't get why we need to keep Math.random at all, since the API is now fully deterministic? Maybe we should just change the name "shuffle" to "reorder" and remove the default. (The "shuffle" in the unit tests are coming from d3-array).
True, I can drop that. In an earlier iteration I allowed passing a random implementation into packEnclose and packSiblings, but then I thought it might break calls to array.map(packSiblings) etc. if we allow the function to take an extra argument. And, I didn’t really want to expose that low-level functionality—I just wanted to ensure that the methods are externally deterministic, while still using “as much randomness as possible” internally. |
Ref. #190 (comment) Supersedes #192.