ThreeTen / threeten

This project was the home of code used to develop a modern date and time library for JDK8. Development has moved to OpenJDK and a separate backport project, threetenbp.
http://threeten.github.io/
191 stars 37 forks source link

ChronoLocalDate generics difficult to use #292

Closed jodastephen closed 11 years ago

jodastephen commented 11 years ago

The signature of ChronoLocalDate with a self type generic causes difficulties AFAICT in actually using the class. See http://hg.openjdk.java.net/threeten/threeten/jdk/rev/47ffbd852f88

This code will not compile:

{
  ChronoLocalDate<?> date = chrono.dateNow();
  date = process(date);
}
private <D extends ChronoLocalDate<D>> D processOK(D date) {
  return date;
}

It does work using a concrete type, such as FooDate.

This seems like pretty basic usage of the API. What it would mean in practice is that no utility methods could be written stating that they return the same CLD type as the type that is input.

I've seen some advice suggesting that wildcards should be avoided in return types, but I'm unconvinced that auto-casting the result is the best solution for methods like dateNow().

I'm beginning to think that the best option may be to remove all generics from CLD/CLDT/CZDT even though that removes functionality from users of the concrete types. At least without the generics, they can be added in JDK 1.9 if a solution can be found.

RogerRiggs commented 11 years ago

The only rough edge is at the interface between wildcards and non-wildcards and can be smoothed by the using the raw type. I'll look into other option with Brian.

RogerRiggs commented 11 years ago

Removing the generics from ChronoLocalDate disables the processOK use case for utility methods.
All methods on ChronoLocalDate would be returning ChronoLocalDate and they won't be compatible with D (without a cast).

jodastephen commented 11 years ago

I tried removing the generics and it basically just worked.

Key methods

- Chronology:
public ChronoLocalDateTime<? extends ChronoLocalDate> localDateTime(TemporalAccessor temporal);
public ChronoZonedDateTime<? extends ChronoLocalDate> zonedDateTime(TemporalAccessor temporal);
public ChronoZonedDateTime<? extends ChronoLocalDate> zonedDateTime(Instant instant, ZoneId zone);
-ChronoLocalDate
public <D extends ChronoLocalDate> ChronoLocalDateTime<D> atTime(LocalTime localTime)

See https://gist.github.com/jodastephen/5302853

RogerRiggs commented 11 years ago

The patch got truncated.

RogerRiggs commented 11 years ago

The JBS issue is JDK-8020280 ChronoLocalDate generics difficult to use

RogerRiggs commented 11 years ago

Resolved by http://hg.openjdk.java.net/threeten/threeten/jdk/rev/1e3f7b402795