LCM of two numbers

Question

I am getting wrong result for my LCM program.

Ifirst find gcd of the numbers and then divide the product with gcd.

int gcd(int x, int y)
{
  while(y != 0)
  {
    int save = y;
    y = x % y;
    x = save;
  }
  return y;
}

int lcm(int x, int y)
{
  int prod = x * y;
  int Gcd = gcd(x,y);
  int lcm = prod / Gcd;

  return lcm;
}

Any help much appreciated.

If you had tested gcd at all, you would have seen that it always returns 0 and the reason for that would have been immediately obvious. Once you had gcd working properly, _then_ would be the time to check that lcm did. This points to a general strategy of software development and debugging. In addition, the fact that this code doesn't even compile is suspicious: how did you get wrong results from it? — Jim Balter, Mar 03 '11 at 05:01
@user642371 In the future, please post your actual code that you have compiled. And responses to comments should contain @name so that the person you are responding to is alerted. Thanks. — Jim Balter, Mar 03 '11 at 05:26

codaddict · Accepted Answer · 2011-03-03T05:16:10.610

Your gcd function will always return 0. Change

return y;

to

return x;

Understand the Euclid's algorithm:

RULE 1: gcd(x,0) = x
RULE 2: gcd(x,y) = gcd(y,x % y)

consider x = 12 and y = 18

  gcd (12, 18)
  = gcd (18, 12)  Using rule 2
  = gcd (12,6)    Using rule 2
  = gcd (6, 0)    Using rule 1
  = 6

As you can see when y becomes zero x will be the gcd so you need to return x and not y.

Also while calculating lcm you are multiplying the numbers first which can cause overflow. Instead you can do:

lcm = x * (y / gcd(x,y))

but if lcm cannot fit in an int you'll have to make it long long

score 4 · Answer 2 · answered Mar 03 '11 at 04:51

4

Problem 1) int gcd = gcd(x,y);

gcd is already defined to be a function. You cannot define a variable with the same name.

Problem 2) Change return y to return x in gcd() otherwise 0 will be returned everytime.

Problem 3) x * y may overflow if x and y are large.

answered Mar 03 '11 at 04:51

Prasoon Saurav

91,295
49
239
345

Thanks you. Problem 1 was a typo. Problem 2 fixed it. – user642371 Mar 03 '11 at 05:00
2

If `x * y` can overflow then so can the result of `lcm`; it's the size of the type of the `lcm` function that is the determining factor. This should be written as `itype lcm(int x, int y) { return (itype)x / gcd(x, y) * y; }` where `itype` is an integer type large enough to hold any expected result. – Jim Balter Mar 03 '11 at 05:21

Priyansh · Answer 3 · 2017-05-24T03:48:13.193

0

#include <iostream>

using namespace std; 

long long gcd(long long int a, long long int b){
    if(b==0)
        return a;
    return gcd(b,a%b);
}

long long lcm(long long a,long long b){
    if(a>b)
        return (a/gcd(a,b))*b;
    else
        return (b/gcd(a,b))*a;
} 

int main(){
    long long int a ,b ;
    cin>>a>>b;
    cout<<lcm(a,b)<<endl;
    return 0;
}

edited May 24 '17 at 03:48

answered May 23 '17 at 18:48

Priyansh

1,163
11
28

score 0 · Answer 4 · answered Mar 03 '11 at 04:52

0

You should return x instead of y in your gcd function.

Also, are you sure the product x*y will always fit into an int? Might be a good idea to use a long long for that as well.

answered Mar 03 '11 at 04:52

MAK

26,140
11
55
86

1

@user642371: It think you already understand why you have to return x and not y. About overflow, suppose both x and y are 2,000,000,000. This fits nicely in an `int` and their LCM is also 2,000,000,000 which is again no problem. But in the intermediate step you compute `x*y` and store it in an `int`. Now, `x*y` is 2,000,000,000*2,000,000,000 which is 4*10^18. This is too big for an `int` and will cause an overflow. You will het an erroneous value in `prod` and dividing it by the gcd will again give you a meaningless value since the original value has been list due to overflow. – MAK Mar 03 '11 at 05:13
2

@user642371 Note that it is not sufficient to put just the product in a long long; you need to make the type of your lcm function a long long to handle all cases. e.g., lcm(2000000000, 2000000001) is 4000000002000000000, which is too large to fit in an int. – Jim Balter Mar 03 '11 at 05:33

score 0 · Answer 5 · answered Aug 12 '18 at 11:52

This C program is different approach towards finding LCM

 #include<stdio.h>
    int main()
    {
        int a,b,lcm=1,i=2;
        printf("Enter two numbers to find LCM\n" );
        scanf("%d %d",&a ,&b);
        while(i <= a*b)
        {
            if(a%i==0 & b%i==0)
            {
                lcm=lcm*i;
                a=a/i;
                b=b/i;
                i=i-1;
            }
            if( a%i==0 & b%i!=0)
            {
                lcm=lcm*i;
                a=a/i;
                i=i-1;
            }
            if( b%i==0 & a%i!=0)
            {
                lcm=lcm*i;
                b=b/i;
                i=i-1;
            }
            i++;
        }
        printf("The LCM of numbers is %d\n", lcm);
    }

LCM of two numbers

5 Answers5

Linked