Skip to content

Rewrite fibonacci.py #5665

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

Closed
tianyizheng02 opened this issue Oct 29, 2021 · 4 comments · Fixed by #5677
Closed

Rewrite fibonacci.py #5665

tianyizheng02 opened this issue Oct 29, 2021 · 4 comments · Fixed by #5677

Comments

@tianyizheng02
Copy link
Contributor

There is a lot of bloat in this file that needs to be cleaned up (empty and unused classes, unnecessary helper functions, inefficient code, etc.)

@Varun270
Copy link
Member

@tianyizheng02 Yeah I agree. So you want a simple straightforward solution of the Fibonacci series with DP right?

@tianyizheng02
Copy link
Contributor Author

No, the original file just had comparisons of different methods of calculating the Fibonacci sequence (iterative and Binet's formula), so those methods just need to be rewritten and cleaned up

@saurabhraj042
Copy link

@tianyizheng02 Can I work on this ?

@johmn123-wq
Copy link

Kindly assign this task to me.

poyea pushed a commit that referenced this issue Oct 31, 2021
* Removed doctest call

* Removed 0 and 1 append to `fib_array`

* Moved fibonacci sequence logic into `calculate`

* Refactored `get` to generate missing numbers

* Renamed `fib_array` to `sequence`

* Renamed `number` to `index`

* Refactored `get` to only return sequence to `index`

* Moved main block into function

* Added documentation to `get`

* Added missing type hints

* Fixed doctest error in `get` docstring

* Moved calculate logic into get

* Reformatted with black

* Fixed wrong generation range
This was referenced Oct 31, 2021
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 a pull request may close this issue.

4 participants