operator == overloading function fails

Question

I am overloading operators > < == in a simple time class.

double exchange_time::seconds_from_midnight() const {
    return seconds + minutes * 60.0 + hours * 3600.0 + milliseconds / 1000.0;
}

bool exchange_time::operator<(const exchange_time& other) const
{
    return seconds_from_midnight() < other.seconds_from_midnight();
}

bool exchange_time::operator>(const exchange_time& other) const
{
    return seconds_from_midnight() > other.seconds_from_midnight();
}

bool exchange_time::operator==(const exchange_time& other) const
{
    return seconds_from_midnight() == other.seconds_from_midnight();
}

The > and < work perfect. However the == yields false and my test fails:

TEST_F(exchange_time_test, comparison) {
    exchange_time et1, et2;
    et1.set("93500001");
    et2.set("93500123");
    EXPECT_TRUE(et2 > et1);
    EXPECT_TRUE(et1 < et2);
    EXPECT_TRUE(et2 == et2);
}

Is there something I'm missing ?

Here's my declaration:

class exchange_time {
    public:
        void set(string timestamp);
        unsigned short int get_milliseconds() { return milliseconds; }
        unsigned short int get_seconds() { return seconds; }
        unsigned short int get_minutes() { return minutes; }
        unsigned short int get_hours() { return hours; }
        double seconds_from_midnight() const;
        bool operator<(const exchange_time& other) const;
        bool operator>(const exchange_time& other) const;
        bool operator==(const exchange_time& other) const;
    private:
        unsigned short int milliseconds;
        unsigned short int seconds;
        unsigned short int minutes;
        unsigned short int hours;
};

`double seconds_from_midnight();` `double`s are imprecise. One could be 1.000000000001 and another 0.9999999999999. More here: [Is floating point math broken?](http://stackoverflow.com/questions/588004/is-floating-point-math-broken) — user4581301, Mar 02 '17 at 17:52
Possible duplicate of [Is floating point math broken?](http://stackoverflow.com/questions/588004/is-floating-point-math-broken) — Richard Critten, Mar 02 '17 at 17:52
Note that all your functions and operators except of `set` should `const`. — Christian Hackl, Mar 02 '17 at 17:54
@user2717954 that doesn't matter; depending on the compiler options and code path, the identical code can deliver subtly different floating point results that don't compare equal. — Mark Ransom, Mar 02 '17 at 17:54
@ChristianHackl not just the functions themselves but their parameters too. — Mark Ransom, Mar 02 '17 at 17:55
@ChristianHackl that might be part of the problem too, the parameters are passed by value but I don't see a copy constructor in the class. — Mark Ransom, Mar 02 '17 at 17:57
How is the provided timestamp translated into hours, minutes, seconds, and milliseconds? How does the `seconds_from_midnight` function work? Please provide a [mcve] — AndyG, Mar 02 '17 at 17:58
`exchange_time` does not need a copy constructor. It is trivially copyable so the compiler provided one will work. — NathanOliver, Mar 02 '17 at 17:58
@MarkRansom: There's the compiler-generated one. This cannot cause the problem, it's just unusual and/or needlessly inefficient. — Christian Hackl, Mar 02 '17 at 17:58
@AndyG the missing pieces shouldn't matter when the object is being compared to itself. The only thing that would matter is if there are members that aren't shown that would make the compiler-generated copy constructor invalid. — Mark Ransom, Mar 02 '17 at 18:01
@MarkRansom: True, I read the comparison as `et1 == et2`, and then nothing made sense. Although, it's possible that `seconds_from_midnight` bizarrely modifies the members. It'd be preferable if we could reproduce it. — AndyG, Mar 02 '17 at 18:14
Are you running on Windows? The compiler may be holding the result of one `seconds_from_midnight` as an 80bit long double, and the other as a 64-bit double - and they won't be equal. Much better to write `uint32_t milliseconds_seconds_since_midnight() const`, and use that. — Martin Bonner supports Monica, Mar 05 '17 at 13:27
@MartinBonner tried your suggestion, still same issue. Almost equal check suggested by silentboy seems to be only solution. — Behrooz Karjoo, Mar 05 '17 at 15:51
Did you notice that I suggested using `uint32_t`? Are you multiplying by integral constants or doubles? Please ask another question *with an [mcve] this time* if you can't get an integral function to work. — Martin Bonner supports Monica, Mar 05 '17 at 15:59
i see. Well seconds_from_midnight can't be int because it has milliseconds as fraction. — Behrooz Karjoo, Mar 05 '17 at 16:54

Shakil Ahamed · Accepted Answer · 2017-03-02T18:20:13.123

4

Never compare equality for double numbers. Check if they are almost equal. The most common way is to use an epsilon to compare the values.

bool exchange_time::operator==(exchange_time other)
{
  return abs(seconds_from_midnight() - other.seconds_from_midnight()) < EPS;
}

Where EPS is a very small value. If you need accurate comparison, you need to define your own Fraction class.

EDIT
EPS stands for Epsilon, which is defined as A very small, insignificant, or negligible quantity of something by Dictionary.com.

edited Mar 02 '17 at 18:20

answered Mar 02 '17 at 18:01

Shakil Ahamed

587
3
18

1

Recommend expanding "eps" to the full word: "epsilon". This will make it much easier for a reader to seek more information with a web search. – user4581301 Mar 02 '17 at 18:08
@user4581301 Ok. I originally added EPS because it is so popular in competitive programming culture that almost everybody uses it almost everyday. I didn't know somebody may complain :D – Shakil Ahamed Mar 02 '17 at 18:17

operator == overloading function fails

1 Answers1