I've written a Rational Class that takes fractions and can add, subtract, multiply, divide etc. I'm trying to keep my fractions reduced but for some reason, some but not all of my fractions are being reduced. I've posted my code below. I commented the fractions that aren't being reduced as //TROUBLE. Why aren't these //TROUBLE fractions being reduced? Is there something wrong with my euclid function or the way I'm calling it? Thanks for your help.

#include <iostream>
using namespace std;

class Rational {
private:
    int Numerator;
    int Denominator;
public:
    Rational(int N = 0, int D = 1)
    {
        this->Numerator = N/euclid(N, D); //Attempt to reduce Numerator
        this->Denominator = D/euclid(N, D); //Attempt to reduce Denominator 
    }
    int euclid(int N, int D) //Euclid Function
    {
        return (D == 0 ? N : euclid(D, N%D));
    } 
    Rational add(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = this->Denominator * Other.Denominator;
        temp.Numerator = (Other.Denominator * this->Numerator) + (this->Denominator * Other.Numerator);
        return temp;
    }
    Rational subtract(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = this->Denominator * Other.Denominator;
        temp.Numerator = (Other.Denominator * this->Numerator) -  (this->Denominator * Other.Numerator);
        return temp;
    }
    Rational multiply(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = this->Denominator * Other.Denominator;
        temp.Numerator = this->Numerator * Other.Numerator;
        return temp;
    }
    Rational divide(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = this->Denominator * Other.Numerator;
        temp.Numerator = this->Numerator * Other.Denominator;
        return temp;
    }
    friend ostream& operator<<(ostream& Output, Rational R) 
    {
        Output << R.Numerator << "/" << R.Denominator;
        return Output;
    }
};

int main() {
    Rational num;

    num = Rational(12, 3);
    cout << "The number is " << num << endl;

    num = Rational(10, 3);
    cout << "The number is " << num << endl;

    num = Rational(35, 40);
    cout << "The number is " << num << endl;

    num = Rational(3, 10).add(Rational(3, 10)); //TROUBLE
    cout << "The number is " << num << endl;

    num = Rational(7, 10).subtract(Rational(3, 10)); //TROUBLE
    cout << "The number is " << num << endl;

    num = Rational(1, 2).multiply(Rational(2, 3)); //TROUBLE
    cout << "The number is " << num << endl;

    num = Rational(1, 2).divide(Rational(2, 3));
    cout << "The number is " << num << endl;
    return 0;
}

It is not so much that there is problem with your euclid function as the fact that you don't call it in add/subtract/multiply/divide thus break OOP rule number 637

A method worth implementing is worth invoking.

Not forgetting

638: A method worth invoking is worth implementing.

and

639: No method is worth implementing twice.

:D

Sorry I've been absent from the thread. You guys were both right though. I made the assumption that since I reduced the Numerator and Denominator in my constructor, the numbers going into the methods would come out reduced like the way they came in. Haha, I was wrong of course. Anyways, here's my working Rational Class. Notice how I call temp.reduce() in each method ;)

#include <iostream>
using namespace std;

class Rational {
private:
    int Numerator;
    int Denominator;
public:
    Rational(int N = 0, int D = 1)
    {
        this->Numerator = N;
        this->Denominator = D;  
        reduce();
    }
    void Rational::reduce()  
    { 
        int GCF;           
        bool Finished=false;   
        if (this->Denominator == 0)
        {
            this->Denominator = 1; 
        }
        if (abs(this->Numerator) > abs(this->Denominator))
        {
            GCF = abs(this->Denominator);
        }
        else
        {
            GCF = abs(this->Numerator);
        }
        while ( !Finished && GCF  >= 2 )
        {
            if ( this->Numerator % GCF == 0 && this->Denominator % GCF == 0) 
            { 
                this->Numerator = this->Numerator / GCF;
                this->Denominator = this->Denominator / GCF;
                Finished = true;
            }
            GCF--;
        }
    }
    Rational add(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = (this->Denominator * Other.Denominator);
        temp.Numerator = (Other.Denominator * this->Numerator) + (this->Denominator * Other.Numerator);
        temp.reduce();
        return temp;
    }
    Rational subtract(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = this->Denominator * Other.Denominator;
        temp.Numerator = (Other.Denominator * this->Numerator) -  (this->Denominator * Other.Numerator);
        temp.reduce();
        return temp;
    }
    Rational multiply(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = (this->Denominator * Other.Denominator);
        temp.Numerator = (this->Numerator * Other.Numerator);
        temp.reduce();
        return temp;
    }
    Rational divide(const Rational &Other)
    {
        Rational temp;
        temp.Denominator = this->Denominator * Other.Numerator;
        temp.Numerator = this->Numerator * Other.Denominator;
        temp.reduce();
        return temp;
    }
    friend ostream& operator<<(ostream& Output, Rational R) 
    {
        Output << R.Numerator << "/" << R.Denominator;
        return Output;
    }
};

int main() {
    Rational num;

    num = Rational(12, 3);
    cout << "The number is " << num << endl;

    num = Rational(10, 3);
    cout << "The number is " << num << endl;

    num = Rational(35, 40);
    cout << "The number is " << num << endl;

    num = Rational(3, 10).add(Rational(3, 10));
    cout << "The number is " << num << endl;

    num = Rational(7, 10).subtract(Rational(3, 10));
    cout << "The number is " << num << endl;

    num = Rational(1, 2).multiply(Rational(2, 3));
    cout << "The number is " << num << endl;

    num = Rational(1, 2).divide(Rational(2, 3));
    cout << "The number is " << num << endl;
    return 0;
}

Thanks guys!
~Nick

Edited 4 Years Ago by NickPatton

This question has already been answered. Start a new discussion instead.