-1

I want to find the greatest common divisor between two input numbers, and I bumped into a problem.

I am not sure if the method I used to actually find the divisor is right. I did it by dividing both of the numbers with every number until the number reaches itself.

#include <iostream>
#include <string>

using namespace std;
    
int main() {

    int num1, num2;
    int large = 0;
    int gcd = 0;

    cout << "this program finds the greatest common divisor" << endl;
    cout << "input first  number > "; cin >> num1; 
    cout << "input second number > "; cin >> num2;

    if (num1 > num2)
        large = num1;
    else
        large = num2;

    cout << "larger number  > " << large << endl;
    cout << endl;

    for (int i = 0; i < large + 1; i++) {
        if (num1 % i == 0 && num2 % i == 0) {
            gcd = i;
        }
    }

    cout << "The gcd of " << num1 << " and " << num2 << " is " << gcd << endl;
    cout << endl;
}
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Danny. Doo
  • 1
  • 2
    Did you test the program? What inputs does it fail on? Also, when `i` is 0, doing `%` with it is not a good idea. – cigien Aug 18 '20 at 13:00
  • 2
    [std::gcd](https://en.cppreference.com/w/cpp/numeric/gcd) Maybe use this – Thrasher Aug 18 '20 at 13:01
  • 1
    You’re not allowed to divide by zero, but it’s the first thing you do. – molbdnilo Aug 18 '20 at 13:04
  • You might try to find which is the most famous of all algorithms. Really, the most famous. Created by a greek philosopher who was born around 325 BC. – gnasher729 Aug 18 '20 at 13:07
  • For a bit more efficient algorithm you can look up in wikipedia: https://en.wikipedia.org/wiki/Euclidean_algorithm#Implementations – rhaport Aug 18 '20 at 13:08
  • Let's assume you use 64 bit integers, and I ask you to find gcd(1, 1000000000000000000). I can do that in my head and give you the answer straight away. Your computer will take years to finish. Why is that? – gnasher729 Aug 18 '20 at 13:10
  • Hello @Danny.Doo! Welcome to StackOverflow. I guess you're learning to program and I would suggest not ask such a question. These are fundamental problems and should be approached yourself. As far as an efficient program is concerned, I would suggest using the Euclidean algorithm for GCD: https://www.freecodecamp.org/news/euclidian-gcd-algorithm-greatest-common-divisor/ – Debargha Roy Aug 18 '20 at 13:10
  • 1
    If you’re looking for the *greatest* common divisor, wouldn’t it make sense to start from the largest potential candidate and work downwards? Then you know that the first one you find is what you’re looking for. (Once you get this working, check out Euclid’s algorithm.) – molbdnilo Aug 18 '20 at 13:10

3 Answers3

0

Yes it makes sense. Basically it does the job. You do few things unnecessary. Start loop:

for (int i = 0; i < large + 1; i++)

with i=1, as you cant div by 0. Also you are performing unnecessary modulo operations when i is between larger and smaller number. Ofc it can be optimized a lot. Obviously on numbers smaller/2 up to smaller you also cant find common divisor(excluding border numbers), and than earlier smaller/3 to smaller/2(-#-). However logic seems solid: last found divisor by the loop will be the gratest common divisor.

Gen0me
  • 115
  • 8
0
#include <iostream> 
using namespace std;

int main() {

    int num_1 = 25;
    cout << "input any number = \n";
    cin >> num_1;
    int num_2 = 15;
    cout << "input any number = \n";
    cin >> num_2;
    int sum = 0;

        for (int i = 1; i <= num_1 && i<=num_2; i++) {
            if (num_1 % i == 0 && num_2 % i == 0) {
                sum = i;
            }
        }
        cout << sum << endl;
        
    return 0;
}
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Hossam Ahmed
  • 1
  • 1
  • 1
    As it’s currently written, your answer is unclear. Please [edit] to add additional details that will help others understand how this addresses the question asked. You can find more information on how to write good answers [in the help center](/help/how-to-answer). – Community Aug 14 '23 at 12:35
-1

You need some modifications to make it correct.
First, check whether one of them is zero, if so, return the other.
Second, if this didn't happen, start dividing from 1, not zero, up to the smallest, not the largest.

int gcd, small;
if (num1 == 0)
    gcd = num2;
else if (num2 == 0)
    gcd = num1;
else
{
    if (num1 < num2)
        small = num1;
    else
        small = num2;
    for (gcd = 1; gcd < small + 1; gcd++)
        if (num1 % gcd == 0 && num2 % gcd == 0)
            break;
}
cout << "The gcd of " << num1 << " and " << num2 << " is " << gcd << endl;

But you should now that there are easier and faster ways, like my answer to this question.

AbdelAziz AbdelLatef
  • 3,650
  • 6
  • 24
  • 52