feat: Added a function for finding the least common multiple #840

tjgurwara99 · 2020-06-11T02:48:49Z

Description of Change

I have added a function that outputs the least common multiple of two numbers.

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.
Sort by alphabetical order
I acknowledge that all my contributions will be made under the project's license.

Notes: Function to find the least common multiple of two numbers.

math/least_common_multiple.cpp

Panquesito7 · 2020-06-12T17:00:38Z

math/least_common_multiple.cpp

@@ -0,0 +1,41 @@
+// Copyright 2020 @author tjgurwara99


Please change this to a comment structure like:

/** * Copyright 2020 @author tjgurwara99 * @file * * (basic description about the program here)

Panquesito7

Everything seems to be good (apart from the above). 👍

ayaankhan98

i suggest you to write some test cases to validate the algorithm properly, please add a function void tests() and write some tests in this functions using assert statements and call this function from main()

tjgurwara99 · 2020-06-12T20:58:01Z

I have taken onboard all the comments and tried to fix the issues, please take a look now.

ayaankhan98 · 2020-06-12T21:24:08Z

@tjgurwara99 have a look at #807 files are very well documented in this PR, please do the documentaion in similar manner.
Thanks! for your contribution

ayaankhan98

looks good

kvedala · 2020-06-13T10:49:16Z

math/least_common_multiple.cpp

+ * @params two integers x and y whose gcd we want to find.
+ * @return greatest common divisor of x and y.
+ */
+unsigned int gcd(unsigned int x, unsigned int y) {


Could you make another pull request in which the functions are templated?

Eg, gcd and lcm Function inputs can be short, int, long, long long, signed or unsigned.

template<typename T> gcd(const T &a, const T &b) {....}

Because it is a recursive function, use of const references gives a faster memory access. If the arguments cannot be made const references, that’s ok

@kvedala I have opened another pull request #845. Once that is complete I'll create a new pull request for these changes.

kvedala · 2020-06-13T10:52:55Z

math/least_common_multiple.cpp

+
+/**
+ * Function for finding greatest common divisor of two numbers.
+ * @params two integers x and y whose gcd we want to find.


The syntax is incorrect please refer #807 and this link

Good documentation, but would not generate correct output.

/** @param <variable name> description @param <Second variable name> description */

kvedala · 2020-06-13T11:08:35Z

math/least_common_multiple.cpp

+/**
+ * Function for finding the least common multiple of two numbers.
+ * @params integer x and y whose lcm we want to find.
+ * @return lcm of x and y using the relation x * y = gcd(x, y) * lcm(x, y)


You can use LaTeX syntax for math formulae in the documentation:

\f$x \cdot y = \mathrm{gcd}(x, y) \cdot /mathrm{lcm}(x, y)\f$

Example output

I didn't know about this feature of Doxygen, thanks for pointing it out!

kvedala · 2020-06-14T14:35:40Z

@tjgurwara99 This is the autogenerated documentation for this file -> link. Note:

the correct tag is param and not params. hence, all the parameter tags got ignored
no \brief tag in the beginning of the file did not create any summary in the list of files page
Thought the points would help you going forward and this was another point I realized should keep an eye while reviewing 😄

tjgurwara99 · 2020-06-14T15:21:45Z

@tjgurwara99 This is the autogenerated documentation for this file -> link. Note:

the correct tag is param and not params. hence, all the parameter tags got ignored

no \brief tag in the beginning of the file did not create any summary in the list of files page
Thought the points would help you going forward and this was another point I realized should keep an eye while reviewing 😄

@kvedala Could you show me the config file (cmake or Doxyfile) you use for Doxygen, I want to try and add that to other projects but don't seem to be able to do it.

kvedala · 2020-06-14T16:46:24Z

@tjgurwara99 This is the autogenerated documentation for this file -> link. Note:

the correct tag is param and not params. hence, all the parameter tags got ignored

no \brief tag in the beginning of the file did not create any summary in the list of files page
Thought the points would help you going forward and this was another point I realized should keep an eye while reviewing 😄

@kvedala Could you show me the config file (cmake or Doxyfile) you use for Doxygen, I want to try and add that to other projects but don't seem to be able to do it.

In my setup, the doxygen gets configured by cmake itself. -> right here

tjgurwara99 added 2 commits June 11, 2020 03:45

feat: Added a function for finding the least common multiple

45e07cf

feat: Miller-Rabin Primality Test (probabilistic)

1e89753

Panquesito7 reviewed Jun 12, 2020

View reviewed changes

Panquesito7 approved these changes Jun 12, 2020

View reviewed changes

ayaankhan98 reviewed Jun 12, 2020

View reviewed changes

tjgurwara99 added 3 commits June 12, 2020 21:47

Added test assertions

356b2fb

Mistakenly worked on a different branch

7e0adcb

Doxygen comments

3faa795

Comments changed

ad8946b

Panquesito7 approved these changes Jun 12, 2020

View reviewed changes

ayaankhan98 approved these changes Jun 12, 2020

View reviewed changes

tjgurwara99 closed this Jun 12, 2020

tjgurwara99 reopened this Jun 12, 2020

ayaankhan98 merged commit 1b5dee7 into TheAlgorithms:master Jun 12, 2020

kvedala reviewed Jun 13, 2020

View reviewed changes

This was referenced Jun 14, 2020

Added algorithm to find lcm (least common multiple). #548

Closed

Lcm of two numbers #526

Closed

kvedala mentioned this pull request Jun 15, 2020

Post-fix Exprssion calculator added #854

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added a function for finding the least common multiple #840

feat: Added a function for finding the least common multiple #840

tjgurwara99 commented Jun 11, 2020

Panquesito7 Jun 12, 2020

Panquesito7 left a comment

ayaankhan98 left a comment

tjgurwara99 commented Jun 12, 2020

ayaankhan98 commented Jun 12, 2020

ayaankhan98 left a comment

kvedala Jun 13, 2020 •

edited

Loading

tjgurwara99 Jun 13, 2020

kvedala Jun 13, 2020 •

edited

Loading

kvedala Jun 13, 2020

tjgurwara99 Jun 13, 2020

kvedala commented Jun 14, 2020

tjgurwara99 commented Jun 14, 2020

kvedala commented Jun 14, 2020

feat: Added a function for finding the least common multiple #840

feat: Added a function for finding the least common multiple #840

Conversation

tjgurwara99 commented Jun 11, 2020

Description of Change

Checklist

Panquesito7 Jun 12, 2020

Choose a reason for hiding this comment

Panquesito7 left a comment

Choose a reason for hiding this comment

ayaankhan98 left a comment

Choose a reason for hiding this comment

tjgurwara99 commented Jun 12, 2020

ayaankhan98 commented Jun 12, 2020

ayaankhan98 left a comment

Choose a reason for hiding this comment

kvedala Jun 13, 2020 • edited Loading

Choose a reason for hiding this comment

tjgurwara99 Jun 13, 2020

Choose a reason for hiding this comment

kvedala Jun 13, 2020 • edited Loading

Choose a reason for hiding this comment

kvedala Jun 13, 2020

Choose a reason for hiding this comment

tjgurwara99 Jun 13, 2020

Choose a reason for hiding this comment

kvedala commented Jun 14, 2020

tjgurwara99 commented Jun 14, 2020

kvedala commented Jun 14, 2020

kvedala Jun 13, 2020 •

edited

Loading

kvedala Jun 13, 2020 •

edited

Loading