ceylon-sdk icon indicating copy to clipboard operation
ceylon-sdk copied to clipboard

ceylon.time.Period compare and equals incompatible

Open dlkw opened this issue 7 years ago • 2 comments

    value p1 = Period(0, 0, 0, 0, 0, 0, 10 * 60 * 60 * 1000);
    value p2 = p1.normalized();
    print(p1<=>p2); // prints "equal"
    print(p1==p2); // prints "false"

According to documentation of ceylon.language.Comparison, the <=> operator (compare method) must return equal if and only if the == (equals method) returns true.

At first sight, it is doubtful that the implementation of the equals method of class Period is correct, because semantically it should make no difference how a period is represented, only how long it takes. This is a bit difficult to decide, as explained in the documentation of the normalized method.

But, presumably, the equals method should also normalize its two operands before comparing the fields.

dlkw avatar Aug 29 '18 17:08 dlkw

Yeah, seems like it could just delegate to ‘compare’.

jvasileff avatar Aug 29 '18 18:08 jvasileff

I think it was a conscious decision at the time not to normalize equals() -- the reasoning at the time was that equals() is (and hash) are very likely to be called quite often in context of collections and normalizing there might become expensive (doubly so considering that common optimization in any comparison operation is first to compare hash and then if hash code matches, consult equals).

But if the language contract says that <=> should return equal if and only if the objects == operator returns true, then I guess, my objections are all moot and this should be fixed.

luolong avatar Aug 30 '18 11:08 luolong