search

trekhleb / javascript-algorithms

📝 Algorithms and data structures implemented in JavaScript with explanations and links to further readings
MIT License
185.25k stars 29.89k forks source link

`isPowerOfTwo` algos don't specify their ranges #889

Open Rudxain opened 2 years ago

Rudxain commented 2 years ago

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?

Yash-Var commented 2 years ago

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

Rudxain commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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