Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

ceylon.time.Period compare and equals incompatible #708

Open
dlkw opened this issue Aug 29, 2018 · 2 comments
Open

ceylon.time.Period compare and equals incompatible #708

dlkw opened this issue Aug 29, 2018 · 2 comments
Assignees

Comments

@dlkw
Copy link
Contributor

dlkw commented Aug 29, 2018

    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.

@jvasileff
Copy link
Contributor

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

@luolong luolong self-assigned this Aug 30, 2018
@luolong
Copy link
Contributor

luolong commented Aug 30, 2018

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants