0

My C++ code includes a class Fraction with the variables "zaehler" (numerator) and "nenner" (denominator). These Fractions should be added, subtracted, multiplied and divided.

I used operator overloading, to do the calculations and the basic combinations work fine and the results are reduced correctly:

enter image description here

When it comes to combining the operators, it mostly worked as expected, but here comes my problem:

enter image description here

The first three are correct, but the combination of multiply and subtracted messes up. While debugging I noticed that one variable seems to have two values:

enter image description here

How can the variables (dif.zaehler and dif.nenner) have different values?

The class and the overloadings (ggT = greatest common divisor, kgV = smallest common denominator):

Fraction::Fraction()
{
    zaehler = 1;
    nenner = 1;
}

unsigned int Fraction::ggT(unsigned int a, unsigned int b)
{
    if (b != 0)     //b = 0, wenn a % b = 0, bedeutet Division ohne Rest
    {
        return ggT(b, a % b);
    }
    else
    {
        return a;
    }
}

unsigned int Fraction::kgV(unsigned int a, unsigned int b)
{
    int facA[30], facB[30];

    for (int i = 0; i < 30; i++)    
    {
        facA[i] = a * (i + 1);
        facB[i] = b * (i + 1);
    }
    for (int i = 0; i < 30; i++)    
    {
        for (int k = 0; k < 30; k++)
        {
            if (facB[k] == facA[i])     
            {
                return facA[i];
            }
        }
    }
}

Fraction Fraction::operator-(const Fraction& b)
{
    Fraction dif;
    int temp_zaehler;



    int zaehler_a = ((float)this->zaehler / this->nenner) * kgV(this->nenner, b.nenner);        
    int zaehler_b = ((float)b.zaehler / b.nenner) * kgV(this->nenner, b.nenner);
    dif.zaehler = (zaehler_a - zaehler_b);
    dif.nenner = kgV(this->nenner, b.nenner);

    if (dif.zaehler < 0)
    {
        temp_zaehler = dif.zaehler;
        dif.zaehler /= ggT(dif.zaehler*-1, dif.nenner);
        dif.nenner /= ggT(temp_zaehler*-1, dif.nenner);
    }
    else
    {
        temp_zaehler = dif.zaehler;
        dif.zaehler /= ggT(dif.zaehler, dif.nenner);
        dif.nenner /= ggT(temp_zaehler, dif.nenner);
    }

    return dif;
}


Fraction Fraction::operator*(const Fraction& b)
{
    Fraction prod;
    int temp_zaehler;
    prod.zaehler = (this->zaehler) * b.zaehler;     
    prod.nenner = (this->nenner) * b.nenner;        


    if (prod.zaehler < 0)
    {
        temp_zaehler = prod.zaehler;
        prod.zaehler /= ggT(prod.zaehler * -1, prod.nenner);
        prod.nenner /= ggT(temp_zaehler * -1, prod.nenner);
    }
    else
    {
        temp_zaehler = prod.zaehler;
        prod.zaehler /= ggT(prod.zaehler, prod.nenner);
        prod.nenner /= ggT(temp_zaehler, prod.nenner);
    }
    return prod;
}

And the line in main.cpp (getString outputs the numerator and denominator as string):

std::cout << "d * e - b = " << d.getString() << " * " << e.getString() << " - " << b.getString() << " = " << (d * e - b).getString() << "\n";
7
  • It looks like you get an integer overflow
    – Ted Lyngmo
    Commented Jun 24, 2023 at 10:25
  • Be careful with implicit conversions between unsigned and signed. The fact that one of the factions is printed as -2 / 5 means that at least the numerator is signed. Converting a int value of -2 to unsigned, which is likely happening implicitly somewhere in your code, yields 4294967294
    – fabian
    Commented Jun 24, 2023 at 10:47
  • Btw: the standard library has std::gcd and std::lcm. Consider using that implementation instead of implemeneting your own...
    – fabian
    Commented Jun 24, 2023 at 10:49
  • 1
    I don't recommend using float btw. This can result in inaccuracies being introduced. Rather than doing int zaehler_a = ((float)this->zaehler / this->nenner) * kgV(this->nenner, b.nenner); ... I'd recommend auto resultDenominator = std::lcm(nenner, b.nenner); auto resultNumerator = (zaehler * (resultDemominator / nenner)) - (b.zaehler * (resultDenominator / b.nenner)); auto factor = std::gcm(resultDenominator, resultNumerator); return {resultNumerator/ factor, resultDenominator/factor}; (you'll still need to make sure to not have any implicit casts from signed to unsigned messing this up...
    – fabian
    Commented Jun 24, 2023 at 11:03
  • 1
    Furthermore I'd recommend implementing the operators at namespace scope to allow implicit conversions to take place. E.g. class Fraction{ ... Fraction(int numerator = 0, int denominator = 1); }; would allow for auto x = 1 + Fraction{1, 2}; with an operator implementation at namespace scope, but your implementation wouldn't allow this. Furthermore default value of arithmetic types is 0 and I recommend you use this convention for your class too, to be consistent with this; initialize to 0 / 1 in the default constructor instead of to 1 / 1...
    – fabian
    Commented Jun 24, 2023 at 11:09

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Browse other questions tagged or ask your own question.