8000 Add more tests for fibonacci by chrisjwelly · Pull Request #36 · TheOdinProject/javascript-exercises · GitHub
[go: up one dir, main page]
Skip to content

Add more tests for fibonacci #36

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
Merged
merged 1 commit into from
Nov 27, 2019
Merged

Add more tests for fibonacci #36

merged 1 commit into from
Nov 27, 2019

Conversation

chrisjwelly
Copy link
Contributor
@chrisjwelly chrisjwelly commented May 24, 2019 •
edited
Loading

I believe some students may be familiar with the recursive definition of the Fibonacci sequence. Instead of using the iterative version, they may come up with a solution like this for a start:

const fibonacci = function(n) {
  if (n === 1 || n === 2) {
    return 1;
  } else {
    return fibonacci(n - 1) + fibonacci(n - 2);
  }
}

This solution will fail the test case where the input is a negative number, but with the given test case for string (fibonacci("8")), it will pass. They may mistakenly think that their solution suffices to cover for all string cases.

However, the only time will this function fails for the string counterpart is when we attempt fibonacci("1") or fibonacci("2"). This will cause a RangeError: Maximum call stack size exceeded due to the fact that at the first iteration, the function attempts to evaluate fibonacci(0), which is not properly handled by this function.

I would like to propose adding these two test cases to cover those writing the recursive definition of Fibonacci.

@codyloyd codyloyd merged commit c2ca1d7 into TheOdinProject:master Nov 27, 2019
willgodev pushed a commit to willgodev/javascript-exercises that referenced this pull request Dec 21, 2020
@codyloyd
Merge pull request TheOdinProject#36 from chrisjwelly/master
e226d4a 
Add more tests for fibonacci
Oussama5379 added a commit to Oussama5379/javascript-exercises that referenced this pull request Feb 1, 2025
@Oussama5379
Merge pull request TheOdinProject#36 from chrisjwelly/master
6241e05 
Add more tests for fibonacci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chrisjwelly @codyloyd
0