[threeten-dev] Please review INSTANT_COMPARATOR rename to timeLineOrder

[Stephen Colebourne]

Apart from an extra space in this line
"public static  Comparator<OffsetDateTime> timeLineOrder() {"
is looks good to me.
Stephen

On 23 May 2013 15:37, roger riggs <roger.riggs at oracle.com> wrote:
> 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
>
>