The Wayback Machine - https://web.archive.org/web/20250520181248/https://github.com/TheAlgorithms/C/pull/736
Skip to content

feat: Added recursive factorial function #736

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

feat: Added recursive factorial function #736

wants to merge 12 commits into from

Conversation

nicola-masarone
Copy link
Contributor

@nicola-masarone nicola-masarone commented Oct 30, 2020 •
edited by gitpod-io bot
Loading

Description of Change

References

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Relevant documentation/comments is changed or added
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

Notes:

Recursive factorial function

Panquesito7
Panquesito7 requested changes Oct 30, 2020
View reviewed changes
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

👍 Good code.

Panquesito7
Panquesito7 requested changes Oct 30, 2020
View reviewed changes
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Please enable GitHub Actions in your repository of this fork in this link: https://github.com/nicola-masarone/C/actions

@github-actions github-actions bot force-pushed the master branch 2 times, most recently from 118f61e to 6d5476a Compare October 31, 2020 08:49
@nicola-masarone
Copy link
Contributor Author

Thanks for all your suggestions.

@nicola-masarone
Update factorial_recursive.c
35a9b51 
Header file explanation and wikipedia link added
@Panquesito7 Panquesito7 changed the title Added recursive factorial function feat: Added recursive factorial function Nov 1, 2020
@Panquesito7 Panquesito7 added Changes requested enhancement New feature or request labels Nov 1, 2020
Panquesito7
Panquesito7 reviewed Nov 2, 2020
View reviewed changes
Panquesito7
Panquesito7 reviewed Nov 2, 2020
View reviewed changes
Panquesito7
Panquesito7 reviewed Nov 2, 2020
View reviewed changes
misc/factorial_recursive.c
* @param val input value to calculate the factorial of
* @returns factorial of the input value
*/
long long factorial_recursive(int val)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
long long factorial_recursive(int val)
long long factorial_recursive(const int& val)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't understand this suggested change. Is there any real need to use const int& instead of just int ?
I supposed this code should be useful for beginners too and I think it's much more readable in this way.
I would prefer to leave it just as it is.
Thanks again for your suggestions.

Copy link
Member

@Panquesito7 Panquesito7 Nov 3, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This parameter (val) is not being modified, it is just being used.
It's better and faster to make it a const and pass it by reference using &.

If the value is being modified, we shall not use const, otherwise it'll throw an error.
Reference.

@github-actions
Copy link
Contributor

This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Oct 13, 2021
@github-actions
Copy link
Contributor

Please ping one of the maintainers once you commit the changes requested or make improvements on the code. If this is not the case and you need some help, feel free to ask for help in our Gitter channel or our Discord server. Thank you for your contributions!

@github-actions github-actions bot closed this Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Panquesito7 Panquesito7 Panquesito7 requested changes

Assignees
No one assigned
Labels
Changes requested enhancement New feature or request Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nicola-masarone @Panquesito7