[threeten-dev] Please review INSTANT_COMPARATOR fix

[Stephen Colebourne]

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