4 ms·
['red', 'blue', 'yellow'].find((d, i) => i === Math.round(Math.random() * 2)) That's not a safe way to pick a random color from an array. `Math.round(Math.ran
by nathan_f77 10y ago
['red', 'blue', 'yellow'].find((d, i) => i === Math.round(Math.random() * 2))
That's not a safe way to pick a random color from an array.
`Math.round(Math.random() * 2)` might produce the following sequence: 2, 2, 0. The find callback is called with 0, 1, 2, so it returns nothing.
Maybe they were going for:
['red', 'blue', 'yellow'][Math.round(Math.random() * 2)]
If they intended for `undefined` or `null` to be a valid result, then it would be better to write:
['red', 'blue', 'yellow', null][Math.round(Math.random() * 3)]
Haha, I submitted a PR: https://github.com/tannerlinsley/react-move/pull/1 https://github.com/tannerlinsley/react-move/pull/1
- vanderZwan 10y agoI suspect you meant to write `Math.floor()` in your second example there? Personally, I just |0 to truncate (still faster in current benchmarks)
- nathan_f77 10y agoHaha oh yeah, `Math.round` was also wrong. Didn't see that one. `|0` is a good trick, I'll have to remember that.
- uniclaude 10y ago|0 also has the benefit of making your code harder to read and look smart. </s> I wouldn't recommend using this unless you're trying to shove milliseconds in this type of operations.
- JSDave 10y agoYou're in the wrong place. If anyone was going for readability, it'd just be _.sample(['red', 'blue', 'yellow']). 0 chance of OP's bug, 0 chance of nathan's bug. Understand exactly what's going on as soon as you finish physically reading it.
- beaconstudios 10y ago> when the senior developer does the code review
- unwind 10y ago*shave, right? Shoving (in) milliseconds would make it slower. :)
- blauditore 10y agoThe non-lambda code is also way more readable in my opinion. I have the impression that people sometimes use lambdas just for the hell of it.