-
-
Notifications
You must be signed in to change notification settings - Fork 116
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.
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
feat(stdlib): isNaN and isFinite utility functions for Number #729
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks great! A couple of notes (aside from the suggested comments):
- We indent the code in the standard library with two spaces, so if you could update these functions to match that style, it would be very helpful.
- With respect to other useful functions, I think an
isInfinite
function (returning true for positive and negative infinity) would round all of this out nicely. This is what other languages (e.g. Python, Racket) appear to do. I'm not strictly opposed toisPositiveInfinity
/isNegativeInfinity
as well, but I am not sure it's necessary when users can doisInfinite(x) && x > 0
. Wondering what @ospencer's thoughts are. - Finally, when making the above change(s), can you rebase this onto the latest
main
?
…variable shadowing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love how readable this is, even to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks awesome! I agree with @peblair that isInfinite
is a fine choice here 👍
Left you some tiny tweaks to your code comments, but otherwise this is perfect!
Co-authored-by: Oscar Spencer <oscar.spen@gmail.com>
Co-authored-by: Oscar Spencer <oscar.spen@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much @cician!
This pull request adds two functions to the number module:
So we want explicit isInfinity and isNegativeInfinity as well?
Internally they depend on isBoxedNumber from runtime/numbers so if you don't mind I'd export it.
Technically there's duplicate code in numberUtils, but it's only for WasmF64 and just two lines of actual code. So I don't know if we want to bother reusing it.
Tests could benefit from waiting for PR #720 I guess.