[threeten-dev] Review #383 ChronoLocalDate generics difficult to use
roger riggs
roger.riggs at oracle.com
Wed Jul 10 07:40:30 PDT 2013
Thanks for the review, I will make the updates.
http://cr.openjdk.java.net/~rriggs/webrev-cld-nogenerictype-292/
I also updated the example code in the chrono package-info.java to
remove the wildcards.
On 7/9/2013 6:54 PM, Stephen Colebourne wrote:
> ChronoLocalDateTimeImpl.plusWithOverflow looks like it should take D
> as the argument, not ChronoLocalDate
right
>
> A number of methods in ChronoZonedDateTimeImpl.ofInstant,
> ChronoZonedDateTimeImpl.ensureValid and
> ChronoLocalDateTimeImpl.ensureValid hide the <D> generic with a
> method-level <D>. The method-level ones should use <R> (for result) to
> be clearer.
fixed
>
> ChronoLocalDateTimeImpl.of() looks like it would benefit from the
> addition of a method-level <R>.
ok
>
> MinguoDate and HijrahDate have a change in readExternal, but Thai and
> Japanese do not.
The change was to consistently return the specific subclass;
previously Minguo and Hijrah returned ChronoLocalDate;
>
>
> None of the above look like vital things to fix before pushing,
> although they should all be investigated at some point.
> Otherwise looks good.
Thanks, Roger
> thanks
> Stephen
>
>
> On 9 July 2013 22:39, roger riggs <roger.riggs at oracle.com> wrote:
>> Please review this change remove the generic type parameter from
>> ChronoLocalDate.
>> As raised in issue #292 <https://github.com/ThreeTen/threeten/issues/292>
>> the type parameter is unnecessary and unwieldy.
>> Initially, it was useful to the implementation of default methods but the
>> implementation
>> has been refactored.
>>
>> This change reduces the number of cases where the developer will need
>> to deal explicitly with wildcard type parameters to ChronoZonedDateTime
>> and ChronoLocalDateTime.
>>
>> The patch contains the changes proposed in the GIST rebased to the current
>> tip.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-cld-nogenerictype-292/
>>
>> Please review and comment.
>>
>> Thanks, Roger
>>
>>
>>
>>
More information about the threeten-dev
mailing list