[threeten-dev] Please review INSTANT_COMPARATOR fix

roger riggs roger.riggs at oracle.com
Wed May 22 14:20:05 PDT 2013


Hi Stephen,

On 5/22/2013 4:14 PM, Stephen Colebourne wrote:
> While I think the fix is correct (as the code looks wrong) the comment
> "compare and INSTANT_COMPARATOR should be the same" is incorrect. The
> whole point of the instant comparator is that it isn't the same as the
> compare method. The test is really about testing nanos when epoch-secs
> are the same.
Right, the statement in the bug report is incorrect. The results may be
different but only in the case where epochSeconds and nanos are equal.
>
> The other changes to Integer.compare should be reverted. They make
> comparisons slower given that in all cases we know that the
> calculation will not overflow.
I expect those comparison functions are intrinsics or are inlined but 
will revert until
it can be demonstrated.

Roger

>
> Stephen
>
>
> On 22 May 2013 20:53, roger riggs <roger.riggs at oracle.com> wrote:
>> Hi,
>>
>> Please review this fix for the issue Patrick found [1] with
>> OfffSetDateTime.INSTANT_COMPARATOR.
>>
>> http://cr.openjdk.java.net/~rriggs/webrev-instant-comparator-8014939/
>>
>> I also noticed that the compareTo methods inconsistently use direct
>> arithmetic
>> and Integer.compare to compute the return values.  The webrev includes
>> updates to consistently use Integer.compare.
>>
>> Thanks, Roger
>>
>> [1] http://mail.openjdk.java.net/pipermail/threeten-dev/2013-May/001401.html
>>



More information about the threeten-dev mailing list