Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

isPowerOfTwo algos don't specify their ranges #889

Open
Rudxain opened this issue May 29, 2022 · 4 comments · May be fixed by #892
Open

isPowerOfTwo algos don't specify their ranges #889

Rudxain opened this issue May 29, 2022 · 4 comments · May be fixed by #892

Comments

@Rudxain
Copy link

Rudxain commented May 29, 2022

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
Copy link

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

@Rudxain
Copy link
Author

Rudxain commented May 31, 2022 •
edited
Loading

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
Copy link
Author

Rudxain commented May 31, 2022

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 Rudxain linked a pull request May 31, 2022 that will close this issue
Open
@Rudxain
Copy link
Author

Rudxain commented May 31, 2022

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Better isPowerOfTwo algos and files Rudxain/javascript-algorithms
2 participants
@Rudxain @Yash-Var