Skip to content

Conversation

vil02
Copy link
Member

@vil02 vil02 commented Sep 4, 2025

Description of Change

factorial_memoization.cpp has a potential segmentation fault: consider the call like fact_recursion(1000).

This PR fixes that and additionally removes one global variable. @git5v: please have a look.

Additionally I decided to remove the math namespace, because it is a cpp file with a main function anyway.

@realstealthninja: regarding the doc-strings: we have different opinion about them. If you will find some additional one useful, please just push it to my branch.

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
  • 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:
Removes a potential segv.

@vil02 vil02 force-pushed the fix_factorial_memoization branch from dcc39b5 to 180b83e Compare September 4, 2025 20:43
@vil02 vil02 force-pushed the fix_factorial_memoization branch from 180b83e to 11347b2 Compare September 4, 2025 20:47
@vil02 vil02 changed the title fix: remove potential segmentation fault from `factorial_memoization.… fix: remove potential segv from factorial_memoization Sep 4, 2025
@vil02 vil02 marked this pull request as ready for review September 4, 2025 20:51
Copy link
Collaborator

@realstealthninja realstealthninja left a comment

Choose a reason for hiding this comment

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

LGTM! Nice spotting

@realstealthninja realstealthninja merged commit 43ce636 into TheAlgorithms:master Sep 5, 2025
6 of 7 checks passed
@vil02 vil02 deleted the fix_factorial_memoization branch September 6, 2025 20:41
realstealthninja pushed a commit to realstealthninja/C-Plus-Plus that referenced this pull request Oct 1, 2025
realstealthninja added a commit to realstealthninja/C-Plus-Plus that referenced this pull request Oct 1, 2025
realstealthninja added a commit to realstealthninja/C-Plus-Plus that referenced this pull request Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants