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.

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

Ports - Ngoc #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Ports - Ngoc #21

wants to merge 2 commits into from

Conversation

lebaongoc
Copy link

No description provided.

Copy link

@mmcknett mmcknett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Checklist

  • Clean, working code
  • Efficient code
  • A detailed explanation of time and space complexity (explains what n stands for, explains why it would be a specific complexity, etc.)
  • All test cases for the assignment should be passing.

lib/fibonacci.rb Outdated Show resolved Hide resolved
if n == 0
return 0
else
p1 = 0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Why p? (Not that you should change it, it's just not obvious to me.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a good point. Do you think naming it p3 would make it easy to understand, since it is the number subsequent to p2 ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was actually curious why you were using p1 and p2 and p3. What's the "p" for?

@@ -5,8 +5,23 @@
# ....
# e.g. 6th fibonacci number is 8

# Time complexity: ?
# Space complexity: ?
# Time complexity: O(n), n is the number of computations this calculation needs to run to compute the nth fibonacci number

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the number of computations this calculation needs to run" is sort of what you're trying to define by explaining the "time complexity" of a method, so you don't want to use that in the definition. "n" as you're using it here is actually the value of the input variable n. The time complexity is n because you're iteratively calculating each fibonacci number up to n.

# Time complexity: ?
# Space complexity: ?
# Time complexity: O(n), n is the number of computations this calculation needs to run to compute the nth fibonacci number
# Space complexity: O(1), space complexity is constant because this method only require a storage for 3 variables (p,p1,p2) regardless of number of operations

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants