Comparing rationals with overloaded operator

Hey I've been trying to compare two rationals and see which one is bigger, but I can't figure out why it's not working. When I put in two rationals like 6/7 and 1/2 it says the second rational is bigger than the first. Everything else works as intend however.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
 #include <iostream>
using namespace std;

class Rational
{
public:

	Rational();
	Rational(int d, int n);
	void ReadRational();
	void display();
	Rational operator +(Rational r)
	{
		Rational temp, temp2;
		if (denom == r.denom)//making sure if both denominators are the same they stay the same
		{
			temp.num = num + r.num;
			temp.denom = denom = r.denom;
			temp.denom = r.denom = denom;
		}
		else
		{
			temp.denom = denom * r.denom;
			temp.num = num * r.denom;
			temp2.num = r.num * denom;//needed another temperoary container otherwise this equation would have overwritten the one above
			temp.num += temp2.num;
		}
		return temp;
		return temp2;
	}
	Rational operator -(Rational r)
	{
		Rational temp, temp2;
		if (denom == r.denom)//making sure if both denominators are the same they stay the same
		{
			temp.num = num - r.num;
			temp.denom = denom = r.denom;
			temp.denom = r.denom = denom;
		}
		else
		{
			temp.denom = denom * r.denom;
			temp.num = num * r.denom;
			temp2.num = r.num * denom;
			temp.num -= temp2.num;
		}
		return temp;
		return temp2;

	}
	Rational operator *(Rational r)
	{
		Rational temp;
		temp.num = num * r.num;
		temp.denom = denom * r.denom;
		return temp;
	}
	Rational operator /(Rational r)//multiplying by the resipricle 
	{
		Rational temp;
		temp.num = num * r.denom;
		temp.denom = denom * r.num;
		return temp;
	}
	bool operator == (Rational r)//this is bool since we are comparing two things and returning it as true or false
	{
		if (num == r.num && denom == r.denom)
		{
			return true;
		}
		return false;
	}
	bool operator >(Rational r)//problem here
	{
		Rational temp, temp2;
		temp.num = num / denom;
		temp.num = r.num / r.denom;
		if (temp.num > temp2.num)
		{
			return true;
		}
		return false;
	}
private:
	int num = 0;
	int denom = 1;

};
Rational::Rational()
{

}
Rational::Rational(int d, int n)
{
	num = 0;
	denom = 1;

}

int main()
{
	int d, n;
	Rational r1, r2;//r in this case will be short for rational
	r1.ReadRational();
	cout << "First rational is ";
	r1.display();//since num and denom are private I call upon this since it is part of the class
	r2.ReadRational();
	cout << "Second rational is ";
	r2.display();
	Rational Sum = r1 + r2;
	cout << "The sum is: ";
	Sum.display();
	Rational Sub = r1 - r2;
	cout << "The subtraction is: ";
	Sub.display();
	Rational multi = r1 * r2;
	cout << "Multiplication is: ";
	multi.display();
	Rational div = r1 / r2;
	cout << "Division is: ";
	div.display();
	if (r1 == r2)
	{
		cout << "Both rationals are the same" << endl;
	}
	else
	{
		cout << "Both rationals are different" << endl;
	}
		
	if (r1 > r2)//problem here
	{
		cout << "The first rational is bigger than the second" << endl;
	}
	else
	{
		cout << "The second rational is bigger than the first " << endl;
	}
		
}
void Rational::display()
{
	cout << num << "/" << denom << endl;
}


void Rational::ReadRational()
{
	cout << "Enter in the numerator: ";
	cin >> num;
	cout << endl;
	cout << "Enter in the denominator (do not put zero in): ";
	cin >> denom;
	if (denom == 0)
	{
		cout << "Invalid input!" << endl;
		exit(1);
	}

}
1
2
3
		temp.num = num / denom;
		temp.num = r.num / r.denom;
		if (temp.num > temp2.num


Spot anything?



Everything else works as intend however.

Depends what "as intend" means for lines 28 and 29 or lines 47 and 48:
1
2
		return temp;
		return temp2;
Last edited on
Also, you can't return two things from a function, your second return statements are unreachable code your in +/- operators. Turn on compiler warnings (-Wall in g++). I think you're confusing yourself by having multiple (perhaps redundant) "temp" variables. Try to avoid that.

Also also, I'm not sure if you ever call this constructor, but read this out loud to yourself:
1
2
3
4
5
6
Rational::Rational(int d, int n)
{
	num = 0;
	denom = 1;

}

Read it physically out loud to yourself. What does the constructor take in as parameters, what does the constructor do in its body?
Last edited on
Sorry for some of those trivial errors. I must of controlled Z too many times when I was working on this problem. As for the returning values problem I still get the right outcome for everything with the exception of comparing the two rationals still. Is that why it doesn't work? Because of the returning of more than one value?
To return a ratio (which is a Numerator + Denominator) you should use std::ratio.

start by including required header:
#include <ratio>

Then redefine your function Rational operator +(Rational r) to return std::ratio something like this:

1
2
3
4
5
6
auto operator +(Rational r)
{
     // other code here...

     return std::ratio(temp, temp2);
}
I tried it your way malibor, but said argument list for class template "std::ratio" is missing
Show us what you have changed the comparison to, @Deadweight77.

Assuming you have corrected the defining of the same thing twice (and the not defining of the other) then you will almost certainly end up suffering from integer division problems.

Avoid them by noting that (if b and d are positive):
a/b > c/d if and only if ad > bc.

If you are allowing negatives in the denominator (I don't recommend it) then you will have to consider the various sign cases.
Here's the compare function:

1
2
3
4
5
6
7
8
9
10
11
bool operator >(Rational r)//problem here
	{
		Rational temp, temp2;
		temp.num = num / denom;
		temp2.num = r.num / r.denom;
		if (temp.num > temp2.num)
		{
			return true;
		}
		return false;
	}
@Deadweight77
the point of std::ratio is that you don't need Rationale class at all.
but if you still want it here example how:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
class Rational
{
public:
	auto operator +(const std::ratio<INT_MAX>& r)
	{
		const auto num = mRatio.num;
		const auto den = mRatio.den;

		std::ratio_add<std::ratio<r.num, r.den>, std::ratio<num, den>> result;

		return result;
	}

private:
	std::ratio<INT_MAX> mRatio;
}


You get the idea, go ahead an implement the rest of the class.

edit:
Please note that if this is your homework, your teacher might suspect you didn't do it yourself but got help from somebody.
Last edited on
@Deadweight77, try

1
2
3
4
    bool operator >(Rational r)
	{
	    return num * r.denom > denom * r.num;
	}


Basic rules of numbers (as in my previous post). For positive b and d:
a/b > c/d if and only if ad > bc.
This will avoid problems with integer division.



I would recommend that, eventually, you should divide numerator and denominator by gcd (greatest common divisor). e.g. 10/8 becomes 5/4. That way you will keep numbers as small as possible and lessen the chances of overflow in some operations.
Last edited on
Topic archived. No new replies allowed.