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 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;
}
``````

## All 3 Replies

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

I would suggest lowest_terms function in accordance to above DRY rule 639 ;)

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 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;

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

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, learning, and sharing knowledge.