Fraction Boolean Operators

Question

I have code to compare two fraction classes, my class operators is below

// Equal to (==)
        bool Fraction::operator == (const Fraction &fraction){
            if (static_cast<double>(numerator / denominator) == static_cast<double>(fraction.numerator / fraction.denominator)){
                return true;
            } else {
                return false;
            }
        } // End of == operator
.
.
.
. // And so on for all other comparisons

However my output returns the (==), (<=) and (>=) operators, saying that they are all true.

In my main()

// EQUAL TO
        if (fraction1 == fraction2){
            cout << fraction1.toString() << " is equal to " << fraction2.toString() << "." << endl;
        }
.
.
. // And so on...

I have also tried if (fraction1.operator==(fraction2)){ instead of if (fraction1 == fraction2) but they both return the same thing. I stuck on this, could there be something wrong with my logic? I can post more code if needed. Thanks!

edit: static_cast should be to a double. The numerators and denominators are integers and in my setter and getter methods they are integers.

Surely basic maths would suggest that the equality condition is `a.numerator * b.denominator == a.denominator * b.numerator`, without division, given that that's the *definition* of a rational number? — Kerrek SB, Nov 30 '15 at 19:11
If numerator and denominator are ints, you could be having some integer division issues. You must cast to floats/doubles *before* dividing (or just use the multiplication technique above). — IanPudney, Nov 30 '15 at 19:13
Also, why don't you step through the ingredient steps and verify that each subexpression actually has the value you may believe it has? — Kerrek SB, Nov 30 '15 at 19:13
And why do anything with the numbers when testing for equality? If both the numerator and denominator are the same then they are equal. — Anon Mail, Nov 30 '15 at 19:15
@AnonMail, it has to also check for equivalent fractions like `3/4==6/8` — R Nar, Nov 30 '15 at 19:16
Oh, I should have specified the numbers passed in are different fractions. (1/2) and (2/3). Since they are both in a class with names `fraction1` and `fraction2` respectively should I be calling those instead? Everytime I try `fraction1`, it just says that identifier `fraction1` is undefined — user3499789, Nov 30 '15 at 19:16
also, why have an if statement if your returns are just booleans, just `return (comparison expression here)` — R Nar, Nov 30 '15 at 19:16
@KerrekSB That's a mistake, I tried to change around the static_cast, Thank you though! — user3499789, Nov 30 '15 at 19:17

shuttle87 · Answer 1 · 2015-11-30T19:31:30.357

if (static_cast<double>(numerator / denominator) == static_cast<float>(fraction.numerator / fraction.denominator)){

The first issue here is that on one you cast to float and other casts to double. This is almost certainly not what you want, firstly you should cast to the same data type for both the things you are comparing. While it might work on in this case because of the bug I outline next you don't want to compare floating point numbers directly, read this https://ece.uwaterloo.ca/~dwharder/NumericalAnalysis/02Numerics/Double/paper.pdf for more information on the pitfalls of doing this.

The big issue I see here is that you have an integer div going on and this will likely blow up badly in some cases, take for example this case where the code fails:

int numerator1 = 5;
int denominator1 = 2;
int numerator2 = 4;
int denominator2 = 2;

In this case you essentially get:

numerator1 / denominator1 == numerator2 / denominator2

Which is probably not what you intended. You can see this in action here http://coliru.stacked-crooked.com/a/0eb3723f952fbf4a

Presuming that you are storing numerator and denominator as integers then you can avoid all floating point numbers in the equality testing by keeping everything as integers by doing:

if (numerator * fraction.denominator == fraction.numerator * denominator){
    return true;
}else{
    return false;
}

This also avoids the bug from earlier. Given you are dealing with rational numbers using some simple algebra you can figure out how to implement the other operations in a similar manner. You just need to check for overflow if you do it this way.

score 1 · Accepted Answer · edited May 23 '17 at 12:15

1

If numerator is less than denominator for all instances of the class that you are using in the call to oparator==(), numerator / denominator will be zero due to integer division.

In that case,

if (static_cast<double>(numerator / denominator) == static_cast<float>(fraction.numerator / fraction.denominator)){

is equivalent to:

if (static_cast<double>(0) == static_cast<float>(0)){

which will be true.

If your numbers are not too large, i.e. there is no risk of integer overflow, you can use:

if ( this->numerator*fraction.denominator == fraction.numerator*this->denominator )

If there is any risk of integer overflow, you can convert the fractions into real numbers but you will need to check for equality using a tolerance.

double f1 = 1.0*numerator/denominator;
double f2 = 1.0*fraction.numerator/fraction.denominator;

if ( fabs(f1-f2) < **SOME TOLERANCE VALUE** )

edited May 23 '17 at 12:15

Community

1
1

answered Nov 30 '15 at 19:20

R Sahu

204,454
14
159
270

Doesn't the tolerance just make it *worse* this time? The test should be as exact as possible after all – harold Nov 30 '15 at 19:21
@harold, that's a judgement call the OP has to make. When comparing floating points numbers, I have found that it usually makes sense to compare them within a tolerance. – R Sahu Nov 30 '15 at 19:25
Because there's usually a build-up of rounding errors. There isn't this time. – harold Nov 30 '15 at 19:26
`if (this->numerator*fraction.denominator == fraction.numerator*this->denominator)` This line worked out perfectly. But in my original code, was it doing integer division and then converting to a double? – user3499789 Nov 30 '15 at 19:28

Fraction Boolean Operators

2 Answers2