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

fix: Adding documentations, tests, and amending algorithm for gcd_of_n_numbers.cpp #2766

Merged
merged 9 commits into from
Oct 7, 2024

Conversation

hollowcrust
Copy link
Contributor

@hollowcrust hollowcrust commented Oct 5, 2024

Description of Change

Adding documentations, tests, and amending the algorithm to align with current guidelines.

Checklist

  • Added description of change
  • 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:
The current algorithm is not accurate and also does not account for 0 in the array, especially when GCD is undefined if all elements are 0.

@realstealthninja
Copy link
Collaborator

please unlink #2456 this does not fix the entirety of that issue. feel free to raise another issue with the name of the algorithm you intend to help update instead.

@realstealthninja realstealthninja added the Improvement improvement in previously written codes label Oct 5, 2024
math/gcd_of_n_numbers.cpp Outdated Show resolved Hide resolved
math/gcd_of_n_numbers.cpp Outdated Show resolved Hide resolved
math/gcd_of_n_numbers.cpp Outdated Show resolved Hide resolved
math/gcd_of_n_numbers.cpp Outdated Show resolved Hide resolved
@realstealthninja realstealthninja added the awaiting modification Do not merge until modifications are made label Oct 5, 2024
@realstealthninja
Copy link
Collaborator

on a side note is this for hacktober fest so i can label it as such

@hollowcrust
Copy link
Contributor Author

Hi yes this is for Hacktoberfest. Thank you :)

Reformatting code, comment and test cases, change array data type.
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.

Lint this file using clang tidy

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

@realstealthninja realstealthninja added approved Approved; waiting for merge hacktoberfest-accepted Accepted to be counted towards Hacktoberfest and removed awaiting modification Do not merge until modifications are made labels Oct 7, 2024
@realstealthninja
Copy link
Collaborator

Thank you for you're contribution. A very well written pr. I will leave this up and if there is no objections will merge in a few days. Best wishes for the rest of hacktoberfest ♥️

@hollowcrust
Copy link
Contributor Author

Thank you for you're contribution. A very well written pr. I will leave this up and if there is no objections will merge in a few days. Best wishes for the rest of hacktoberfest ♥️

Thanks!

@realstealthninja realstealthninja merged commit fddedd8 into TheAlgorithms:master Oct 7, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved; waiting for merge hacktoberfest hacktoberfest-accepted Accepted to be counted towards Hacktoberfest Improvement improvement in previously written codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants