[threeten-dev] Please review INSTANT_COMPARATOR rename to timeLineOrder

[roger riggs]

Please review this change to rename OffsetDateTime.INSTANT_COMPARATOR
to be consistent with ChronoZonedDateTime.timelineOrder().

http://cr.openjdk.java.net/~rriggs/webrev-instant-comparator-8015293/

Thanks, Roger



On 5/23/2013 9:22 AM, Stephen Colebourne wrote:
> On 23 May 2013 14:13, roger riggs <roger.riggs at oracle.com> wrote:
>> A followup while examining the OffsetDateTime comparator...
>>
>> The compareTo method contains an optimization for the offsets being equal.
>> Should that optimization also be in INSTANT_COMPARATOR?
>>
>> With lambda syntax for method references, I created a static 2 arg method
>> for compareInstant and used it for the INSTANT_COMPARATOR
>> and in the compareTo method avoiding duplicate code.
>>
>> In your opinion would it be useful to make the static comparator method
>> public,
>> perhaps instead of the INSTANT_COMPARATOR field.
>>
>> http://cr.openjdk.java.net/~rriggs/webrev-instant-comparator-8014939/
> I think that sharing the code as you have done is a good idea.
> However, I wouldn't make the method public.
>
> What I would recommend is changing INSTANT_COMPARATOR constant to be
> timeLineOrder() static method to match ChronoZDT (for consistency).
>
>
>> Also, to the previous comment, should the use of Long.compare be changed
>> to arithmetic subtraction using the previous argument?
> Ideally, everywhere where you previously changed, and the cases in
> ODT, where (a - b) is used instead of Integer.compare should have an
> end of line comment such as // safe from overflow to document the
> deliberate design decision.
>
> Stephen