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

Humble disagreement on straightforward recursive fibonacci implementation being "divide and conquer" #17

Closed
Rulikkk opened this issue Jun 4, 2019 · 4 comments

Comments

@Rulikkk
Copy link

Rulikkk commented Jun 4, 2019

I think that binary search is a better representative of "divide and conquer" approach.

Even more, I suggest removing straightforward recursive fibonacci implementation from that group — it seems to belong to group "never do like this" (unless you have some cool recursion optimisation in your language).

The reason is, instead of "dividing and conquering", the problem is actually multiplied to two problems on each step (and one problem is equivalent to another one from previous step).

Maybe code like this would be closer to demo "divide and conquer", using fibonacci and recursion.

@amejiarosario
Copy link
Owner

amejiarosario commented Jun 4, 2019

Thanks for raising your concern @Rulikkk. I see your points, maybe in the GitHub repo is harder to follow the sequence. Let me share my answers about your comments:

I suggest removing straightforward recursive Fibonacci implementation from that group — it seems to belong to group "never do like this" (unless you have some cool recursion optimization in your language).

This performance issue is on purpose. It's actually the nexus for the next topic Dynamic Programming and the new fib implementation.

image

I think that binary search is a better representative of the "divide and conquer" approach.
I agree there are many candidates for divide and conquer and we cover others as well.

image

Maybe code like this would be closer to demo "divide and conquer", using Fibonacci and recursion.

In the same stack overflow thread, you can see that use one of the implementations called memoization, very similar to the one we used on this repo

@amejiarosario
Copy link
Owner

Let me know if you still have questions or suggestions

@Rulikkk
Copy link
Author

Rulikkk commented Jun 6, 2019

I'm cool. I've mostly referred to this section of README: https://github.com/amejiarosario/dsa.js#algorithms

If that's not the core part—it's OK.

@amejiarosario
Copy link
Owner

Ah, that's a good point. In the readme, I should put another example since there's not much context. Thanks for pointing that out!

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

No branches or pull requests

2 participants