Overflow of answer for larger values

Question

i am trying to find the LCM of a number using the following formula. Lcm = Gcd/(a*b). This is working fine for small number, however for larger numbers it overflows, like the one shown in the code. I tried using long long as the variable type but still no effect. How do i fix the overflow issue?

#include <iostream>
#include <vector>
using namespace std;

long long int LCM(int n1, int n2){

    const int size = 2;
    long long int sum;
    long long int gcd;
    long long int lcm = 0;
    vector<int> number(2);
    number[0] = n1;
    number[1] = n2;

while (true)
{
    sum = number[0] % number[1];
    gcd = number[1];
    if (sum == 0)
        break;
    number[0] = number[1];
    number[1] = sum;
}

lcm = ((n1*n2)/gcd);
return lcm;
}

int main()
{

    cout << LCM(28851538, 1183019) << endl;
    system("pause");

}

By using even larger data types :) `unsigned long long` for example, or have a look at GMP or Boost — Rakete1111, Aug 19 '16 at 19:13
You can try using `unsigned long long`. This is type can hold greater numbers than `long long`. — ForceBru, Aug 19 '16 at 19:13
In your case, by replacing `(n1*n2) / gcd` with `(n1 / gcd) * n2`. For even larger numbers: Use an arbitrary precision integer library. — dhke, Aug 19 '16 at 19:15

score 4 · Answer 1 · answered Aug 19 '16 at 19:15

There's a trivial improvement.

You calculate (n1 * n2) / gcd. That will overflow if n1 * n2 is too large to fit into an int. One obvious change would be to calculate ((long long) n1 * (long long) n2) / gcd. That is fine as long as n1 * n2 is not too large to fit into long long.

But assume you want to use this function with long long arguments. Then remember that gcd is the greatest common divisor of n1 and n2. So it's a divisor of n1 and a divisor of n2. So you calculate (n1 / gcd) * n2 or (n2 / gcd) * n1, which will give the same result. There will be no overflow unless the final result is too big.

So just change the return statement to

return (n1 / gcd) * n2;

score 2 · Answer 2 · answered Aug 19 '16 at 19:15

2

Since you know that gcd divides evenly into both numbers, simply change the order of the operations:

lcm = n1*(n2/gcd);

answered Aug 19 '16 at 19:15

Mark Ransom

299,747
42
398
622

score 0 · Answer 3 · answered Aug 19 '16 at 19:18

0

long long int LCM(int n1, int n2)

parameters are int!

vector<int> number(2)

why int again ?

lcm = ((n1*n2)/gcd)

use n1/gcd * n2

answered Aug 19 '16 at 19:18

bluemmb

85
1
9

Overflow of answer for larger values

3 Answers3