define contracts for equality in ceylon.time

[jvasileff]

While working on https://github.com/ceylon/ceylon/issues/6261 I discovered that there are several places in ceylon.time where == is used but not defined. I think it would improve the api to make the meaning of == clear.

The cases can be broken into two categories.

Equality for Date, Time, and TimeZone

My suggest is to add

shared actual formal Boolean equals(Object other);

for each of these, with documentation defining a contract for equality for these types (and, of course, to check implementations and uses).

Equality for UnitOfYear and UnitOfHour

Are these simply enumerated types? If so, my suggestion is to

1. add satisfies Identifiable for each of them, and
2. make the leaf interfaces (e.g. UnitOfYear UnitOfHour) enumerated or sealed

[DiegoCoronel]

Equality for Date, Time, and TimeZone My suggest is to add shared actual formal Boolean equals(Object other);

I dont understand, Date, Timeand ZoneDateTimeare interfaces, I dont believe the contract should enforce equals as this must be a implementation detail... also our implementations are refining equals. Can you help me to understand why do you think we should enforce equals implementation?

Equality for UnitOfYear and UnitOfHour Are these simply enumerated types? If so, my suggestion is to

• add satisfies Identifiable for each of them, and
• make the leaf interfaces (e.g. UnitOfYear UnitOfHour) enumerated or sealed

They are interfaces and used as enumerated types... we have object implementations available as utilities, but I think we should not make it sealed, I believe they are usefull for others frameworks.

or I get it wrong your view?

[lucaswerkmeister]

You’re already testing equality on those interfaces, so you’re relying on them properly implementing equals (you shouldn’t test “implementation details” IMO – if you change the implementation while keeping the external contract, the tests should still work). It makes sense to document how these equals functions behave.

[DiegoCoronel]

Ah, right, I got this @lucaswerkmeister , tks :)

[jvasileff]

Without defining equality for Date, it's impossible to say if this program is correct:

shared void foo(Date today, Date birthday) {
    if (a == b) {
        print("It's your birthday!");
    } else {
        print("It's not your birthday.");
    }
}

Does equality depend on the particular Date implementation? Or, must all subclasses be able to compare with each other, to determine equality based on some general definition defined by Date?

If it's the first, and each implementation is free to define it's own equals() and never return true when passed an instance of a different implementation, then a == b in the program above is a bug.

Edit: attempt to improve the example code.

[DiegoCoronel]

@jvasileff tks for your explanation, I believe i got it, and about the second point ? about enumerated?

[jvasileff]

@DiegoCoronel I suspect that if you allow others to satisfy UnitOfYear, and they define their own myYear object, then code that uses == may break, since year != myYear. But I'll admit I don't know the API well enough to be sure about this.

Either way, it seems like it would be worth explicitly stating how == should work for UnitOfYear and UnitOfHour, since == is being used on instances of these types.

[DiegoCoronel]

Ok, tks for your patience explaining @jvasileff, I would like to see @luolong points about this issue. Then Im going to implement something and open a MR and of course as always he will review my code ;)

[luolong]

I somewhat agree on the point of defining equality contracts for Date/Time/DateTime/ZoneDateTime equality.

The only question here is, should we define the contract on the level of Date interface or should we go one step deeper and define the equality based on ReadableDate?

I personally keep bouncing between the two opinions. On one hand the ReadableDate is the interface that defines the shape of a date and it makes sense to define equality based on that.

On the other hand, ReadableDate is satisfied by ReadableDateTime and comparing those two may be ambiguous and non-symmetric, so this does not really make too much sense.

At the moment, after this little bit of rubber-ducking, I tend to lean more towards defining equality on Date (and friends), allowing implementations to override it if they can provide more efficient implementation (like we currently have in the case of GregorianDate).

On the topic of UnitOfYear and UnitOfHour, I need to review those classes and their usage.

[gavinking]

@jvasileff @DiegoCoronel @luolong in case it doesn't make sense to have a concrete implementation of equals() on the interface, it is a good practice to refine and redeclare equals() as formal on the interface, along with documentation explaining the semantics attached to equality for instances of that interface.

Collection is an example of this pattern.

[luolong]

Okay. makes sense.

[jvasileff]

in case it doesn't make sense to have a concrete implementation of equals() on the interface, it is a good practice to refine and redeclare equals() as formal on the interface, along with documentation explaining the semantics attached to equality for instances of that interface.

@gavinking I don't necessarily agree with this. Although having formal equals() would help organize documentation, declaring a method to explicitly state that equality isn't defined across subtypes shouldn't be necessary; that's already true by default.

Lists can't "just" choose to be equal to Sets without equality from Sets to Lists somehow being defined. Explicitly stating this in a super type doesn't change anything.

Collection is an example of this pattern

Fortunately, though, this is only done in the interface documentation, not as an override. Otherwise, my proposal in https://github.com/ceylon/ceylon/issues/6261#issuecomment-217920838 wouldn't work so well for collections.

(Of course I agree that if equality does exist across subtypes, equals should be defined.)

[DiegoCoronel]

@jvasileff do you agree with the fix ?? can we/you close this issue ?

[jvasileff]

@DiegoCoronel thanks, I think the changes look good.

Any thoughts on equality for TimeZone? The use of TimeZone.equals() in question is in ZoneDateTime.equals():

source/ceylon/time/timezone/ZoneDateTime.ceylon:31: warning: dangerous ==
                && timeZone == other.timeZone;

Since equality for TimeZone isn't documented, I'm not exactly sure of the meaning of ZoneDateTime.equals().

---

Also, was any decision made on equality for the units? I still haven't looked closely at these, so I don't really have an opinion. The uses of == in question are:

source/ceylon/time/DateRange.ceylon:135: warning: dangerous ==
        return step == this.step then this else DateRange(from, to, step);
               ^
source/ceylon/time/DateTimeRange.ceylon:134: warning: dangerous ==
        return step == this.step then this 
               ^
source/ceylon/time/TimeRange.ceylon:134: warning: dangerous ==
        return step == this.step then this else TimeRange(from, to, step);
               ^

[DiegoCoronel]

@jvasileff right, let me take a look yet this week, tks

[luolong]

@jvasileff, @DiegoCoronel the equality of TimeZone has not really been tackled yet.

The thing is that we have two types of time zones in ceylon.time:

• Fixed offset time zones
• Rule based time zones (essentially time zones that should have all the magic of DST rules from tzdb)

Currently only zones with fixed offset have been properly implemented. The equality there is fairly simple and could be easily defined based on the offset.

Rule based time zones are a bit more complex beasts. Luckily, this part of the API is yet to be implemented, so I guess equality of those is also open.

Offset timezones and rule based time zones should never be considered equal.

[luolong]

@DiegoCoronel I can't really remember the reasoning for defining the units of time and date as interfaces.

For what it's worth, it seems to me that the equality should be based on the identity of the constants.

I see no reason for UnitOfYear, and others having any other alternative implementations aside from the ones defined in ceylon/time/base/constants.ceylon

[jvasileff]

@luolong would it make sense to avoid the question, and define ZoneDateTime.equals() solely in terms of instant? It seems like this is what I would expect anyway, although I'll admit I could be missing a use case.

[luolong]

Time zone adds a semantic detail to the instant, that could be important to the application. Otherwise why use ZoneDateTime instead of Instant?

[jvasileff]

Fair enough. And, it looks like java.time.ZoneDateTime compares time zones too.

[DiegoCoronel]

@DiegoCoronel I can't really remember the reasoning for defining the units of time and date as interfaces. For what it's worth, it seems to me that the equality should be based on the identity of the constants. I see no reason for UnitOfYear, and others having any other alternative implementations aside from the ones defined in ceylon/time/base/constants.ceylon

I believe it was because i wasnt able to use switch case for object ... so, at time I chose interfaces, nothing especial about this choice