Skip to content

adding factorial - #930

Merged
cclauss merged 3 commits into
TheAlgorithms:masterfrom
jpg-130:factorial
Jul 17, 2019
Merged

adding factorial#930
cclauss merged 3 commits into
TheAlgorithms:masterfrom
jpg-130:factorial

Conversation

@jpg-130

Copy link
Copy Markdown
Contributor

calculating factorial using memoization added

@shahabmohammadishahabmohammadi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good , the code working correctly and i think could explain the idea of developer very easy.

@cclauss

Copy link
Copy Markdown
Member

Could you please add a doctest or two?

@jpg-130

Copy link
Copy Markdown
ContributorAuthor

It is showing that the travis-ci build failed, however, it works well on the local machine. What changes should I incorporate?

@cclauss

Copy link
Copy Markdown
Member

https://travis-ci.org/TheAlgorithms/Python/builds/558476077#L705
Think like a compiler would. Is the variable spelled correctly? Is it in scope?

@cclauss

Copy link
Copy Markdown
Member

Need another hint?

@jpg-130

jpg-130 commented Jul 14, 2019

Copy link
Copy Markdown
ContributorAuthor

@cclauss I'll try. Thanks!

Comment threaddynamic_programming/factorial.py Outdated
if __name__ == "__main__":
import doctest
result=[-1]*10
result[0]=result[1]=1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hint: Move lines 33 and 34 inside the factorial() function.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok but if I do that the values are recomputed every time the function is called i.e. I don't want to recompute 3! if 7! is already computed.

@cclausscclaussJul 16, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. Then put those two lines at the top of the file at global scopebefore the function and the main()

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank You!

@cclausscclauss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@cclauss
cclauss merged commit f195d92 into TheAlgorithms:masterJul 17, 2019
@jpg-130
jpg-130 deleted the factorial branch July 17, 2019 07:06
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
* adding factorial
* adding doctest
* Update factorial.py
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jpg-130@cclauss@shahabmohammadi