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@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@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