javascript-algorithms icon indicating copy to clipboard operation
javascript-algorithms copied to clipboard

`isPowerOfTwo` algos don't specify their ranges

Open Rudxain opened this issue 2 years ago • 4 comments

Neither the README nor the src files explicitly say that the bitwise approach is only correct for 32bit ints. The way it's worded implies that the bitwise approach is always better, but the "naive" approach is the most mathematically correct, it even handles NaN and Infinity correctly! Why isn't there a clarification?

Rudxain avatar May 29 '22 09:05 Rudxain

@Rudxain I want to fix this issue can you assign it to me .....

Yash-Var avatar May 30 '22 13:05 Yash-Var

Can you assign it to me

I don't have any authority to do that lol. You could open a PR and write "Fixes #889" or "Closes #889" to auto-close this Issue if it gets merged

Rudxain avatar May 31 '22 01:05 Rudxain

Wait I realized there's a bigger Issue. The bitwise isPowerOfTwo is duped, an instance is in the math/bits folder, and the other in math/is-power-of-two. This makes no sense. The bitwise variant isn't mathematical, it should be exclusively located in the bits directory, not directly within math. I'm gonna open a PR

Rudxain avatar May 31 '22 02:05 Rudxain

@Yash-Var I've already opened a PR. If you want to contribute you can send PRs to my fork, or open an independent PR that solves this Issue in a different way

Rudxain avatar May 31 '22 02:05 Rudxain