tags:

views:

295

answers:

6

Ok, so I'm trying to create a program using a while loop to find the greatest common divisor of two numbers. This is what I came up with. However, from what I can tell, the program just seems to skip the loop entirely when I run it. (opers remains 0, divisor always comes back as equal to num1). Anyone out there that can help out a newbie?

/* Define variables for divisors and number of operations */

int num1, num2, divisor, opers;
opers = 0;

/* Prompt user for integers and accept input */

cout << "Please enter two integers with the smaller number first, separated by a space. ";
cout << endl;
cin >> num1 >> num2;

/* Make divisor the smaller of the two numbers */

divisor = num1;

/* While loop to calculate greatest common divisor and number of calculations */

while ( (num1 % divisor != 0 ) && ( num2 % divisor != 0 ) )
{

   divisor--;
   opers++;
}

/* Output results and number of calculations performed */

cout << "The greatest common divisor of " << num1 << " and " << num2 << " is: ";
cout << divisor << endl << "Number of operations performed: " << opers;
+6  A: 

As soon as one of those modulo returns non 0, the while loop terminates. (So if any of your inputs immediately results in 0 from the modulo, the loop won't be entered)

What you probably want:

while ( (num1 % divisor != 0 ) || ( num2 % divisor != 0 ) )
{

   divisor--;
   opers++;
}

This continues the loop until both modulo operations result in 0.

Doug T.
dotjoe
oooo. Reminds me if Electrical Engineering 101. Notted input and gate is equivalent to notted output or gate.
Martin York
+1  A: 

divisor == num1 initially, so (num1 % divisior != 0) is not true.

dicroce
+1  A: 

num1 == divisor so num1 % divisor == 0 and the loop condition is false. You want to use || instead of &&.

You probably also want to use a better algorithm. I think Euclid came up with one.

Dave Hinton
A: 

num1 = divisor:

5/5 = 1

so this (num1 % divisor != 0 ) evaluates always to true and the other doesn't, you won't ever enter.

Lex
+1  A: 

It doesn't work because your algorithm is wrong! For a proper GCD algorithm, see here.

rlbond
Wrong or sub-optimal?
Bill
+1  A: 

The other users have a good point. I just want to add that since you're starting out you should learn some simple ways to help debug and find problems with your code. One very common tool beginners use is print statements. If you add print statements in key areas then you can find the problems pretty easily.

cout << "Please enter two integers with the smaller number first, separated by a space. ";
cout << endl;
cin >> num1 >> num2;

/* Make divisor the smaller of the two numbers */

divisor = num1;

cout << "Checking values ..." << endl;
cout << "num1 = " << num1 << endl;
cout << "num2 = " << num2 << endl;
cout << "divisor = " << divisor << endl;

/* While loop to calculate greatest common divisor and number of calculations */

cout << "about to start loop" << endl;
while ( (num1 % divisor != 0 ) && ( num2 % divisor != 0 ) )
{

   divisor--;
   opers++;
   cout << "In the loop and divisor = " << divisor << " and opers = " << opers << end;
}
cout << "after loop" << endl;

So you could do the output however you want but this is just to show the idea behind it. I hope this helps you in future debugging. Also, there are actual debugging programs that are much more advanced than this method; but this works for simple isssues.

BobbyShaftoe