ChronoLocalDate generics difficult to use #292

Closed
jodastephen opened this Issue Mar 31, 2013 · 6 comments

Projects

None yet

2 participants

@jodastephen
Member

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
Collaborator

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 RogerRiggs was assigned Apr 1, 2013
@RogerRiggs
Collaborator

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
Member

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
Collaborator

The patch got truncated.

@RogerRiggs
Collaborator

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

@RogerRiggs RogerRiggs closed this Jul 10, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment