-1

I want to subtract two fractional numbers using operator overloading. I have write a piece of code in order to accomplish this task:

#include<iostream>
using namespace std;

void HCF(int& a, int& b)
{
    int m, n;

    m = a;
    n = b;

    while (m != n)
    {
        if (m > n)
            m = m - n;
        else
            n = n - m;
    }
    a = a / m;
    b = b / m;
}

class Rational {
    int x1;
    int y1;
public:
    void simplify()
    {
        int m, n, r;
        n = fabs(y1);
        m = fabs(x1);
        while (r = m % n)//Find the Maximum Common Number of m,n
        {
            m = n;
            n = r;
        }
        y1 /= n; // Simplification
        x1 /= n;
        if (y1 < 0) // Convert denominator to positive number
        {
            y1 = -y1;
            x1 = -x1;
        }
    }

    Rational(int num = 0, int denom = 1)
    {
        if (denom) {
            x1 = num;
            y1 = denom;
        }
        else {
            x1 = 0;
            y1 = 1;
        }
    }

    Rational(const Rational& copy)//Copy Constructor
    {
        x1 = copy.x1;
        y1 = copy.y1;

    }

    Rational operator-(const Rational& x) const //Overloaded minus operator
    {
        Rational temp;
        temp.x1 = x1 * x.y1 - x.x1 * y1;
        temp.y1 = y1 * x.y1;
        temp.simplify();
        return temp;
    }

    operator string() const //Overloaded string operator
    {
        int numerator = x1, denominator = y1;
        HCF(numerator, denominator);
        string str;
        if (denominator == 1)
            str = to_string(numerator);
        else
            str = to_string(numerator) + "/" + to_string(denominator);
        return str;
    }
};

int main()
{
    Rational a(5, 1);
    Rational b(3, 4);
    Rational c;
    Rational d;
    Rational x(5, 1);
    c = a - x;
    string expected1 = "0"; //Expected value
    string actual1 = (string)c;//Actual value

    cout << actual1.compare(expected1);//Comparing actual and expected value
    d = c - b;
    string expected2 = "-3/4";
    string actual2 = (string)d;

    cout << actual2.compare(expected2);
}

As we can see, in the main() function, both cout statements should print 0 because when we compare both strings, they must be equal. But the problem is, when I run this program, it prints nothing, and I don't know why.

My operator string() converts an integer fraction into a string. My operator- takes the LCM and finds the final value after subtracting two fractional numbers. In the main() function, I have declared five objects for class Rational, and then I simply perform subtraction and then I compare the expected and actual values, which should return 0 in the cout statement.

I have also used the simplify() member function to check whether the fraction is completely in simpler form or not. For example, it should simplify 2/4 into 1/2.

Where is the mistake? All other functions except operator- are running correctly. The real problem is with the operator-, ie when we perform c=a-x, the value of c would be 0 as a string. Similarly, if we perform d=c-b then the expected value should be -3/4 as a string.

I have also tried to run this code on Google Test, but it fails the test case.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • `while (r = m % n)` -- is the assignment of `r` here intended? – Fred Larson Apr 19 '22 at 20:09
  • I didn't get you. Can you kindly explain? – Mudassir Waheed Apr 19 '22 at 20:12
  • What it does is actually find the maximum common number of m and n. – Mudassir Waheed Apr 19 '22 at 20:14
  • That `while` condition is not _comparing_ `r` to `m % n`, it is _assigning_ it. A comparison would be `while (r == m % n)`. This is a common gotcha. It may be what you intend, but I'm not certain. – Fred Larson Apr 19 '22 at 20:14
  • 3
    What happens, if you pass `0` as the first parameter to `HCF`? – fabian Apr 19 '22 at 20:14
  • 1
    Some handy reading: [What are the basic rules and idioms for operator overloading?](https://stackoverflow.com/questions/4421706) One of its recommendations is that + and - not be class methods. – user4581301 Apr 19 '22 at 20:14
  • 2
    Side note: `Rational` manages no resources that require special handling, so the [Rule of Zero](https://en.cppreference.com/w/cpp/language/rule_of_three) suggests that you `default` or leave out the copy constructor. – user4581301 Apr 19 '22 at 20:17
  • No its the logic I made. I didn't inted to write == instead =. – Mudassir Waheed Apr 19 '22 at 20:20
  • 1
    Also why implement the same logic (simplifying the faction) twice? Also why explicitly take the floating point version of the absolute value function even though the input is `int` and the output is cast back to `int`? – fabian Apr 19 '22 at 20:22
  • 1
    What do you want `HCF(0, 1)` to return? Right now you have an endless loop. – Wyck Apr 19 '22 at 20:28
  • What changes should i need to make in HCF function? – Mudassir Waheed Apr 19 '22 at 20:45
  • @OmegaCoding Since you claim that the `compare()` calls are not returning 0, have you tried, oh, I don't know, outputting the *actual* values to see what they really are? Have you even tried to [debug thins code](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) yet at all? – Remy Lebeau Apr 19 '22 at 21:12

1 Answers1

1

Use this GCD (greatest common divisor) function to implement your HCF function. Yours currently exhibits an endless loop when given 0 as a first argument.

To reduce a fraction, divide numerator and denominator by their greatest common divisor.

This is called the Euclidean algorithm.

template <typename T>
T GCD(T a, T b) {
    while (b != 0)
    {
        T t = a % b;
        a = b;
        b = t;
    }
    return std::max(a, -a);
}
Wyck
  • 10,311
  • 6
  • 39
  • 60