-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
added fibonacci using formula along with test cases #1358
added fibonacci using formula along with test cases #1358
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.
Please add this to the existing Fibonacci.js
file (and add the tests to the corresponding test file). Use a proper JSDoc comment. Also, use each
for the test cases.
The formula doesn't need to be a comment, that's redundant with the implementation (also, the formula in the comment seems to differ from the one you implemented?). You can use x**y
instead of Math.pow(x, y)
. Note also that Wikipedia suggests a slightly simpler formula for computation by rounding.
Side note: I find calling this O(1) time/space complexity slightly misleading; this is only (trivially) given because the numbers involved have constant size. If this were to use bigints / bigdecimals, it wouldn't be constant time anymore.
@appgurueu When I'll be able to merge this PR ? |
It's still waiting on a second review from @raklaptudirm. |
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.
Sorry for the late review, but consider adding a resource to the formula used here.
know more
Describe your change:
I added a code which calculates the nth fibonacci number using formula in a very less time.
Checklist:
Example:
UserProfile.js
is allowed butuserprofile.js
,Userprofile.js
,user-Profile.js
,userProfile.js
are notFixes: #{$ISSUE_NO}
.