-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
algorithm: signum #1266
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
algorithm: signum #1266
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.
Looks fine; note: this will currently return 1
for NaN; returning NaN would be more sensible IMO.
Oh yes. I completely missed that case. I'm sorry. Thanks for pointing it out, I'll fix it right away.
You still have an else
in each branch. Generally using else
-if
s even if you are returning on each branch is not necessarily bad practice because it may help improve readability (so I'm fine with it either way), but in this particular example, your if
s are simple enough that you should just get rid of the redundant else
s as the return
s already do the job.
Uh oh!
There was an error while loading. Please reload this page.
Open in Gitpod know more
Describe your change:
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.This PR is to add the implementation of the signum function. TheAlgorithms entry is here