foundations/object_basics/03_fibonacci/solution/fibonacci-solution.js: Fix solution error handling for fibonacci() when count is NaN

[Vairiation]

Because

Previous logic for solution allows fibonacci('hello') to return 1 instead of 'OOPS'. While this is not a breaking change, I felt as though it was relevant when looking at the conditional logic.

This PR

• Add !count to if conditional block that returns "OOPS" so it now reads if (!count || count < 0) return "OOPS"
• Move if (count == 0) return 0 before if (!count || count < 0) return "OOPS" to allow proper handling of count = 0

Issue

Closes #XXXXX

Additional Information

Pull Request Requirements

• [ x] I have thoroughly read and understand The Odin Project Contributing Guide
• [ x] The title of this PR follows the location of change: brief description of change format, e.g. 01_helloWorld: Update test cases
• [ x] The Because section summarizes the reason for this PR
• [ x] The This PR section has a bullet point list describing the changes in this PR
• [ x] If this PR addresses an open issue, it is linked in the Issue section
• [ x] If this PR includes any changes that affect the solution of an exercise, I've also updated the solution in the /solutions folder

[mao-sz]

Thanks for suggesting this. I'm not actually convinced by it though. The point of the exercise isn't to make a robust function for real world usage that covers most/all possible cases, but rather to just practise some fundamentals in order to pass all the tests, for which NaN handling isn't.

That being said, looking at the current tests, I'm not a fan of the last few that deal with strings. Not really seeing the point having them in this exercise specifically; there are plenty of opportunities for parsing/converting strings to numbers throughout the whole curriculum.

---

I'd like to propose a different and slightly larger change to this exercise. If you're happy to do it, then just do the work in this branch. Otherwise, you can close the PR and I'll get the work handled another way.

I think instead of testing for string->number conversion, we should amend the instructions and tests so that if the argument is a negative number, NaN or not a number-type at all, return "OOPS", i.e. only proceed with positive numbers.

Means it focuses more on doing the fibonacci stuff and just shoving aside all other types of values, no other type-specific behaviour.

[Vairiation]

Hey mao-sz, I just made the requested changes to the solution function, solution test suite, and student test suite.

I added in a check for NaN specifically since its type is a number and could slip through checking typeof countArg !== 'number' alone.

Let me know if that was the direction you were looking for. Also, this is my first pull request on a non-individual repo so let me know if I did something incorrectly.

[Vairiation]

I think that should be everything from your suggestions. Let me know if I missed anything!

[mao-sz]

The test changes need to be duplicated across the solution test file as well