Hey I'm trying to calculate GCD (greatest common denominator) with cpp but my program is printing 0 instead of the GCD and there is no error!!

#include <iostream>

using namespace std;

int main()
{
    int emptyi;
    int number1;
    int number2;
    int counter1 = 0;
    int baghimandeh1 = 0;
    int liste1[100];
    int baghimandeh2 = 0;
    int counter2 = 0;
    int liste2[100];

    cout << "add your number: " << endl;
    cin >> number1;

    cout << "add your second number: " << endl;
    cin >> number2;

    for (int i = 1; i <= number1; i  ) {
        if (number1%i == 0) {
            baghimandeh1 = number1;
            liste1[counter1] = i;
        }
        counter1  ;
        // baghimandeh1 = number1 %i ;
    }

    for (int i = 1; i <= number2; i  ) {
        if (number1%i == 0)
            baghimandeh2 = number2;
        liste2[counter2] = i;
    }
    counter2  ;

    // baghimandeh2 = number2 %i ;

    for (int i = 0; i <= counter1; i  ) {
        for (int j = 0; j <= counter2; j  ) {
            if (liste1[i] == liste2[j]) {
                emptyi = liste1[i];
            }
        }
    }

    cout << "b,m,m  shoma hast: " << emptyi;
}

I wanna calculate Denominator of each user's input and then print the biggest among them and it will be GCD but it print's 0 no matter what number I enter.

CodePudding user response：

I'll point out some errors I've seen:

• counter1 and counter2 should be updated only when the if clauses are true.
• The block below always executes the last line; I understand this is a bug you introduced when debugging.
• Also in that block, you use number1 instead of number2.

if (number1%i == 0)
    baghimandeh2 = number2;
liste2[counter2] = i;

Apart from those errors:

• The algorithm only works with input numbers with no more than 100 divisors.
• Some variables are not initialized.
• The last double for loop is very inefficient.
• It would be preferable not to using namespace std;

Here's a demo of your code with some fixes and some debug info added.

The algorithm could be much simpler though; a bit more efficient and without needing to use any array. For example, given two positive numbers, get the mininum of them, and walk an index from that minimum towards 1, until both numbers are divisible by the index.

[Demo]

#include <algorithm>  // min
#include <iostream>  // cin, cout

int main() {
    int number1{};
    std::cout << "add your number:\n";
    std::cin >> number1;

    int number2{};
    std::cout << "add your second number:\n";
    std::cin >> number2;

    auto min_counters{std::min(number1, number2)};

    int emptyi{};
    for (int i = min_counters; i >= 1; --i) {
        if (number1 % i == 0 and number2 % i == 0) {
            emptyi = i;
            break;
        }
    }

    std::cout << "b,m,m shoma hast: " << emptyi;
}

CodePudding user response：

#include <iostream>
#include <vector>

int main()
{
    int number1;
    int number2;
    std::vector<int> liste1, liste2;

    std::cout << "add your number: " << std::endl;
    std::cin >> number1;

    std::cout << "add your second number: " << std::endl;
    std::cin >> number2;

    for (int i = 1; i <= number1; i  ) {
        if (number1 % i == 0) liste1.push_back(i);
    }

    for (int i = 1; i <= number2; i  ) {
        if (number2 % i == 0) liste2.push_back(i);
    }

    auto d1 = liste1.rbegin();
    auto d2 = liste2.rbegin();
    while(true) {
        if (*d1 == *d2) {
            std::cout << "GDC(" << number1 << ", " << number2 << ") = " << *d1 << std::endl;
            break;
        }
        if (*d1 > *d2) {
              d1;
        } else {
              d2;
        }
    }
}

Note: testing all integers for number2 is inefficient. You only need to test those numbers that are a denominator for number1, i.e. everything in liste1. So just iterate backwards over liste1 till you find a number that is a denominator for number2.