[threeten-dev] [threeten-develop] Reconsidering generics on Chrono classes

[Stephen Colebourne]

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.

Every date(...) factory and similar in Chronology has to be overridden
(as in IsoChronology). Not all are yet in HijrahChronology

Similar comments for other chronologies.

While other eras are unimportant, there might be a case to make
JapaneseEra into a public class.

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).

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?

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
>
>
>
>
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_d2d_jan
>
>
>
> _______________________________________________
> threeten-develop mailing list
> threeten-develop at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>
>
> --
> Thanks, Roger
>
> Oracle Java Platform Group
>
> Oracle is committed to developing practices and products that help protect the environment
>
>
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_d2d_jan
> _______________________________________________
> threeten-develop mailing list
> threeten-develop at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>