TheOdinProject / curriculum

The open curriculum for learning web development
https://www.theodinproject.com/
Other
9.99k stars 13.35k forks source link

Advanced Javascript/project Recursion: error in the fibonacci exercise #25687

Closed lorenzomad closed 11 months ago

lorenzomad commented 1 year ago

Describe your suggestion

In the fibonacci exercise there are 2 parts:

  1. Using iteration, write a function fibs which takes a number and returns an array containing that many numbers from the fibonacci sequence. Using an example input of 8, this function should return the array [0, 1, 1, 2, 3, 5, 8, 13].
  2. Now write another function fibsRec which solves the same problem recursively. This can be done in just a couple of lines (or 1 if you’re crazy, but don’t consider either of these lengths a requirement… just get it done).

There is a mistake in part 2, since the second part should not state to do the same with recursion. As visible in the solution linked below, the recursive solution only gives as output the nth element of the fibonacci sequence, not the whole sequence up to n.

I hope the difference is clear from my explanation. I spent quite some time trying to solve it before looking at the solution and understanding the different result expected.

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/javascript-recursion

Checks

(Optional) Discord Name

No response

(Optional) Additional Comments

No response

CouchofTomato commented 1 year ago

Hey @lorenzomad

Thanks for raising this issue.

You mention a solution linked below but I don't see anything linked?

The first two student submissions in the list seem to solve the recursive part the same way as the first part so I'm curious what you're referring to?

lorenzomad commented 1 year ago

Sorry if I was not clear: when I mentioned the solution, I mean the solution linked in the lesson, which is the following: https://medium.com/launch-school/recursive-fibonnaci-method-explained-d82215c5498e

As you can see the link gives a solution to obtain the nth element of the sequence, not the full sequence up to n, which in my opinion is much harder than something you do in 1 or 2 lines of code as stated in the exercise script.

To answer your other point, the first 2 solutions do that, but the second student "cheated" (let me used the term in a non-serious manner) by adding the previous list as an input, while the exercise states that the function should only take 1 number as input. The first student actually managed to do it, but in my opinion they implemented a quite complex solution with a lot of if/else states which is not exactly what the course required:

        ? "Please enter a valid number of elements to be given an answer."
        : n === 1
        ? [0]
        : n === 2
        ? [0, 1]
        : [...fibsRecurse(n - 1), fibsRecurse(n - 1)[n - 2] + fibsRecurse(n - 1)[n - 3]];

I believe the level of the previous recursion exercises was instead referring to finding the solution also linked in the medium link which is this:

def fibonacci(number)
  if number < 2
    number
  else
    fibonacci(number - 1) + fibonacci(number - 2)
  end
end

If you think I am wrong the issue can be closed however

CouchofTomato commented 1 year ago

Thanks @lorenzomad

I'll ask someone to take a look at it

CouchofTomato commented 1 year ago

Hey @lorenzomad

I've discussed this with the team. We think it makes the most sense if the two assignments for fibonacci have the same expected output, and therefore feel we'd be better suited trying to find an updated resource rather than change the assignment. Is this something you want to do? Or do you want us to open it up for someone else to work on?

lorenzomad commented 1 year ago

Is it possible to discuss changing the first exercise to output the nth number of the fibonacci sequence? I think that would be less tedious. If that is not what the team wants then I can try to look for an update resource.

CouchofTomato commented 1 year ago

Any thoughts on this @ManonLef

ManonLef commented 1 year ago

@CouchofTomato

I'm not married to this way of doing things, but I think getting this extra practice of recursive array storage could be useful. Mostly since compared to the more complex assignments coming up, here we have relatively little code to work with. Later on in BST for example, if people want to store elements recursively, the code is probably already a bit bulkier.

On the other hand, we'd probably lower the difficulty of this exercise quite a bit if we remove it and let both exercises console log the sequence.

That last bit could be a bonus or not, depending on how you look at it. I personally think this exercise as it is, poses challenges, which when overcome are very helpful in the upcoming harder assignments. They are also proof of a good understanding of the chain of events and resolving inside the recursion. If we expect a lot of people are giving up on this exercise (I do note a lot of questions in discord but not people giving up on it afaik) then the learning value would be negative.

I'm mostly neutral, but slightly leaning towards keeping it the way it is.

CouchofTomato commented 1 year ago

Thanks @ManonLef

I think as things stand we have to decide between the two options but if we do ever expand the course we could possibly offer both options at two different points in the course. Similar to how html has beginner and intermediate sections we could do the same for CS.

I was sort of leaning towards changing it, because this is an introductory course so I wonder if we're ramping up the difficulty too soon.

See if I can russle up some more opinions on this.

wise-king-sullyman commented 1 year ago

I largely share @ManonLef's thoughts. While I definitely remember this exercise being quite the challenge it's nothing compared to the projects that follow not long after it.

If in the future we expand CS though I wouldn't be opposed to us breaking things into a recursion one and two and having this be in the latter though.

CouchofTomato commented 1 year ago

Ok I don't think we have the support for the change, so I'm closing this for now. Thanks for the contribution though @lorenzomad

ManonLef commented 1 year ago

If we do want an alternative article explanation it might be worth looking into these articles:

There are more to be found online.

@CouchofTomato @lorenzomad what do you think? Look for a different link or leave it as is (since it was mentioned earlier in the conversation)

CouchofTomato commented 1 year ago

@ManonLef

The last example doesn't explain a solution for the problem we're asking students to solve. It's using a recursive function to calculate the nth term but calling it inside a loop and just printing that number. This is the issue we'll find I think. Most resources on tackling this problem from the perspective of calculating the nth term rather than returning an array of the fibonacci series.

I'll open this issue back up for now because there is still the question of the resource but I don't have an easy solution.

ManonLef commented 1 year ago

@CouchofTomato Oh you're right! I was glancing at the memorization option it seems and it seemingly using an array. That was hasty of me.

github-actions[bot] commented 1 year ago

This issue is stale because it has had no activity for the last 30 days.

ManonLef commented 1 year ago

I'm working on a new resource for this

keyBash commented 1 year ago

yes. that will help

github-actions[bot] commented 1 year ago

This issue is stale because it has had no activity for the last 30 days.

ManonLef commented 1 year ago

Still got this :) It needs some more focus time from me!

github-actions[bot] commented 1 year ago

This issue is stale because it has had no activity for the last 30 days.

ManonLef commented 12 months ago

I happened to come across this article on Fibonacci which does both with and without array storage.

No matter how much I'd love to finish my own article, and I still intend to do so in the future, it has not been up high on my priority list since it does require quite a bit of mental resources and animation creativity brainwidth.

Would love to hear what you think about it @CouchofTomato

CouchofTomato commented 12 months ago

@ManonLef

Yeah this looks a pretty comprehensive article. Definitely better than what we have I think.

ManonLef commented 11 months ago

Thanks @CouchofTomato @lorenzomad would you still like to add this?

lorenzomad commented 11 months ago

Thanks for finding the article! I can take care of changing the link in the course material.

lorenzomad commented 11 months ago

Created PR with link to new article