CODEWITHSUNDEEP
cfunctionreturnreturn-value
Celery
Celery

Reputation: 109

C: Function call not returning correct value

My function "euclid" correctly computes the expected return value (f2 in the if statement), but when it returns from its call in the main, "gcd = euclid(factor_one, factor_two);" is not correct.

Example: with the current #s as the factors, 97, 13, it should return 1, which is what f2 is equal to, but when I print gcd, it says it is 0.

What is my error?

int euclid(int f1, int f2);

int main()
{
    int factor_one = 97, factor_two = 13;
    int gcd;
    gcd = euclid(factor_one, factor_two);
    //gcd = factor_one % factor_two;
    printf("GCD = %d\n",gcd );
}

int euclid(int f1, int f2)
{
   if (f1%f2 == 0)
   {
        //printf("base case %d \n", f2);
        printf("GCD = %d\n",f2 );
        return f2;
   }
   else
   {
       int temp = f1%f2;
       //printf("%d\n", temp);
       euclid(f2, temp);
   }
}

Upvotes: 1

Views: 1535

Answers (2)

bin
bin

Reputation: 724

When I run the code in my machine it gave correct output. The answer I got 1 as you said. The program is correct. Check the compiler you used. Some compiler ask to return value for even main function and few don't ask.

Better you use recursive function because it will drastically reduce your code as well as execution time.

Upvotes: 0

Jonathan Leffler
Jonathan Leffler

Reputation: 754820

Converting comments into an answer.

  1. You need to return the value from the recursive call: return euclid(f2, temp);.

  2. You probably can simplify the code with int temp = f1 % f2; before the condition; then if (temp == 0) { … } else { return euclid(f2, temp); }.

  3. You should only print in the euclid() function as a debugging measure.

  4. The recursion in the proposed fixed code is tail recursion. It can be replaced by iteration.

And Alan Au gave the sage suggestion:

  1. General advice: always a good idea to turn on compiler warnings with -Wall. It would have told you the problem in this case with this: warning: control reaches end of non-void function [-Wreturn-type]

These suggestions yield a recursive solution:

int euclid(int f1, int f2)
{
    int temp = f1%f2;
    if (temp == 0)
    {
        //printf("base case %d \n", f2);
        //printf("GCD = %d\n",f2 );
        return f2;
    }
    else
    {
        //printf("%d\n", temp);
        return euclid(f2, temp);
    }
}

and an iterative solution:

int euclid(int f1, int f2)
{
    int temp = f1%f2;
    while ((temp = f1 % f2) != 0)
    {
        f1 = f2;
        f2 = temp;
    }
    return f2;
}

Upvotes: 2

Related Questions