[threeten-dev] Reconsidering generics on Chrono classes

[Roger Riggs]

Hi,

Cleaned up the methods on the Chrono and calendar types and updated the 
javadoc <http://cr.openjdk.java.net/%7Erriggs/javadoc-chrono-generics-191/>.

But the generics on ChronoLocalDate will need to be restored.
Without  them it is not possible to write a library than can return
the same type of date passed in.    Something as simple as:

<D extends ChronoLocalDate>  D next(D date) {
!    D next = date.plus(2, DAYS);
     return next;
}


fails because the type of date.plus is always ChronoLocalDate.

More tomorrow,   Roger



On 02/03/2013 08:09 PM, Roger Riggs wrote:
> M7 is destined for the developer preview and after that even minor 
> changes will
> not be welcome and we're out of time.
>
> So I'd push for the major change to be in M7.
>
> I will cut the public APIs in the public date classes back to the 
> public methods
> in ChronoLocalDate* to avoid extra design work and testing since we
> had agreement that ChronoLocalDate was functionally sufficient.
>
> BTW, there are a few stray static methods in Chronology to support 
> date.atTime()
> and creating ChronoZonedDateTime instances.  With static methods in 
> interfaces
> is there a better place for them?
>
> On 02/03/2013 07:25 PM, Stephen Colebourne wrote:
>> I'm guessing that this won't be pushed tomorrow for any immediate
>> freeze, as it needs a fair set of work/review. But I think it has
>> potential to be cleaner for users (but since I'm not a direct user of
>> this code its harder for me to tell). It certainly makes the code look
>> cleaner.
>>
>> HijrahDate has public methods like plusDays/minusDays that you've
>> overridden to return the right type. But those methods are not part of
>> the current public API of ChronoLocalDate. One of the advantages of
>> having a real class is that we can add these methods publicly, but we
>> don't have to as part of this change.
>>
>> HijrahEra has "(HijrahDate)(getChronology().date(this, year, month,
>> day));" which should be  "getChronology().date(this, year, month,
>> day);" if the getChronology() method returns the correct type.
> Since Chronology is no longer generic it cannot return the correct type.
> The runtime type is correct but not the declared type.
> The Era code should be using the factories in the Date classes and
> won't have to go through the Chronology.  The current cast was quicker to
> get compiled.
>> Every date(...) factory and similar in Chronology has to be overridden
>> (as in IsoChronology). Not all are yet in HijrahChronology
> Yes
>> Similar comments for other chronologies.
>>
>> While other eras are unimportant, there might be a case to make
>> JapaneseEra into a public class.
> I'd leave it be.
>> There might also be a case for removing getEra() from ChronoLocalDate
>> as it adds less value now (it could still be on JapaneseDate and
>> others where it matters).
> The era is still needed to be able to pass to Chronology to create dates.
>> In ChronoLocalDate the atTime method returns a raw type
>> ChronoLocalDateTime. This would need to be ChronoLocalDateTime<?>  as
>> we can't have raw types. I don't know if ChronoLocalDateTime<FooDate>
>> is a valid covariant return type override for ChronoLocalDateTime<?>,
>> (and that is the key question as to whether this works).
>>
>> Does having double the number of public types affect the package name question?
> No that has not been an issue.  If it were twice as many....
>
> Thanks, Roger
>
>> thanks for looking at this,
>> Stephen
>>
>>
>>
>>
>> On 3 February 2013 22:33, Roger Riggs<Roger.Riggs at oracle.com>  wrote:
>>> Update the javadoc with the suggestion to remove generics from
>>> Era, ChronoLocalDate, and Chronology.
>>>
>>>      http://cr.openjdk.java.net/~rriggs/javadoc-chrono-generics-191/
>>> webrev:
>>>      http://cr.openjdk.java.net/~rriggs/webrev-chrono-generics-191/
>>>
>>> Still work in progress, the samples have not been updated
>>> and the javadoc may still refer to generic parameters in places.
>>>
>>> Roger
>>>
>>>
>>>
>>>
>>>
>>> On 2/3/2013 8:59 AM, Roger Riggs wrote:
>>>
>>> On 02/02/2013 06:26 PM, Stephen Colebourne wrote:
>>>
>>> I've taken a quick look. It seems that the generics have changed from
>>> Chronology to date class, which strikes me as different rather than
>>> better. Can you give any examples where this makes user code simpler?
>>> And why you see it as an improvement?
>>>
>>> The move to generics around the date types makes the code written
>>> easier to read without bloating the api with separate types of DT, ZDT.
>>>
>>> My thoughts in #191 were trying to remove generics entirely from
>>> Chronology and Era. My theory is that a user will work in one of three
>>> ways
>>> a) only with ISO, and thus with concrete types
>>> b) only with a non-ISO calendar system, and thus a public XxxDate class
>>> c) only at the abstract chrono-level
>>>
>>> A single user with a small application might have the luxury of working in only
>>> one of those modes at their own choice.  But real systems are mixes of imported
>>> libraries and utilities.
>>> And even a single developer may find value in developing utilities that are re-usable.
>>>
>>> If there is a public XxxDate class, then (a) and (b) users are happy
>>> (with the exception of (b) user accessing
>>> ChronoLocalDateTime/ChronoZonedDateTime)
>>>
>>> The generics are useful in this case to avoid a forced shift between b and c.
>>>
>>> The real question is whether those (c) users working with the abstract
>>> chrono-level care about whether the calendar system object they are
>>> passing around is the same as that of another object (which either
>>> form form generics allows). The question would seem partly answered by
>>> j.u.Calendar, which doesn't restrict or generify by calendar system.
>>>
>>> At the interface between an application wanting to write a calendar specific
>>> application and a library providing calendar independent  functions the
>>> generics provide type checking at the integration point.  If the library provides
>>> return types parameterized by the date types, the caller does not have
>>> to drop down to the abstract chrono API to handle the returned datetime values.
>>>
>>> We also have Oracle Globalization team comments that they would always
>>> create a date in the abstract, without knowledge of the calendar
>>> system. Given this, there is no way to ever populate the generic
>>> argument from typical chrono-level usage. Thus whether the generic it
>>> is a Chronology or a ChronoLocalDate is irrelevant.
>>>
>>> They also recognize the value  of being able to use both models.
>>>
>>> No generics on any of the five key classes (Chronology, Era,
>>> ChronoLocalDate, ChronoLocalDateTime, ChronoZonedDateTime) requires
>>> three public classes for each calendar system, which seems overkill.
>>>
>>> Yes, this is one reason to retain the generics .
>>>
>>> A more practical/balanced approach would be:
>>> - Chronology - no generics
>>> - Era - no generics
>>> - ChronoLocalDate - no generics
>>> - ChronoLocalDateTime<D extends ChronoLocalDate>
>>> - ChronoZonedDateTime<D extends ChronoLocalDate>
>>>
>>> With this approach, chrono-level users (c) would use ungenerified
>>> types Chronology/Era/ChronoLocalDate and wildcarded
>>> ChronoLocalDateTime<?>  and ChronoZonedDateTime<?>. Whereas (b) users
>>> would use XxxChronology, Era, XxxDate, ChronoLocalDateTime<XxxDate>
>>> and ChronoZonedDateTime<XxxDate>.
>>>
>>> For me, the question here was whether this approach could be made to
>>> work, as it seemed to be the approach that met the needs of users (a),
>>> (b) and (c) best.
>>>
>>> If Chronology is not parameterized, there would be cases where it would be
>>> desirable to pass in a particular known chronology and get back a date-time
>>> object known to be of that chronology.
>>>
>>> Perhaps a minor point, but without generics on ChronoLocalDate many
>>> methods would need to be overridden to return the correct type.
>>>
>>> I'll spend some more time this afternoon looking at this.
>>>
>>> Thanks, Roger
>>>
>>> Stephen
>>>
>>>
>>> On 2 February 2013 21:43, Roger Riggs<Roger.Riggs at oracle.com>  wrote:
>>>
>>> Hi,
>>>
>>> I took another look at the use of generics in the Chrono classes and
>>> drafted an alternative that is an improvement.
>>>
>>> Issue #191 mentions that the generics do not add (enough) value.
>>>
>>> Changing the parameter from Chronology to the concrete ChronoLocalDate type
>>> is more usable for the case where the concrete date type is being used
>>> deliberately
>>> and enable greater fluency.
>>>
>>> Exposing the concrete date types along with the Chronology types makes
>>> it easier
>>> to write calendar specific applications and makes code using
>>> ChronoLocalDateTime
>>> and ChronoZonedDateTime more fluent.
>>>
>>> Take a look at the comment.
>>> javadoc:
>>>      http://cr.openjdk.java.net/~rriggs/javadoc-chrono-generics-191/
>>> webrev:
>>>      http://cr.openjdk.java.net/~rriggs/webrev-chrono-generics-191/
>>>
>>> This is not ready to commit, though the functionality of the local date
>>> classes is stable, the javadoc needs improvement.
>>> In particular, the exposure of useful methods in the javadoc of each class
>>> is uneven since some are inherited and only appear in the inherited section
>>> of the javadoc.
>>>
>>> Thanks, Roger
>>>
>