Joe,

The identity check in CS.compare makes sense.  However, it won't be null 
hostile if we call CS.compare(null, null) and that doesn't seem right.
Usually when writing comparator classes I end up with:
===
if (Objects.requireNonNull(o1) == Objects.requireNonNull(o2)) {
    return 0;
}
===

Jason
________________________________________
From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> on behalf of Joe 
Wang <huizhe.w...@oracle.com>
Sent: Friday, February 2, 2018 1:01 PM
To: core-libs-dev
Subject: Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, 
StringBuilder, and StringBuffer

Hi,

Thanks all for comments and suggestions. I've updated the webrev. Please
review.

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 1/31/2018 9:31 PM, Joe Wang wrote:
> Hi Tagir,
>
> Thanks for the comment. I will consider adding that to the javadoc
> emphasizing that the comparison is performed from 0 to length() - 1 of
> the two sequences.
>
> Best,
> Joe
>
> On 1/29/18, 8:07 PM, Tagir Valeev wrote:
>> Hello!
>>
>> An AbstractStringBuilder#compareTo implementation is wrong. You cannot
>> simply compare the whole byte array. Here's the test-case:
>>
>> public class Test {
>>    public static void main(String[] args) {
>>      StringBuilder sb1 = new StringBuilder("test1");
>>      StringBuilder sb2 = new StringBuilder("test2");
>>      sb1.setLength(4);
>>      sb2.setLength(4);
>>      System.out.println(sb1.compareTo(sb2));
>> System.out.println(sb1.toString().compareTo(sb2.toString()));
>>    }
>> }
>>
>> We truncated the stringbuilders making their content equal, so
>> sb1.toString().compareTo(sb2.toString()) is 0, but compareTo compares
>> the original content (before the truncation) as truncation, of course,
>> does not zero the truncated bytes, neither does it reallocate the
>> array (unless explicitly asked via trimToSize).
>>
>> With best regards,
>> Tagir Valeev.
>>
>>
>> On Fri, Jan 26, 2018 at 10:00 AM, Joe Wang<huizhe.w...@oracle.com> 
>> wrote:
>>> Hi,
>>>
>>> Adding methods for comparing CharSequence, StringBuilder, and
>>> StringBuffer.
>>>
>>> The Comparable implementations for StringBuilder/Buffer are similar
>>> to that
>>> of String, allowing comparison operations between two
>>> StringBuilders/Buffers, e.g.
>>> aStringBuilder.compareTo(anotherStringBuilder).
>>> For CharSequence however, refer to the comments in JIRA, a static
>>> method
>>> 'compare' is added instead of implementing the Comparable interface.
>>> This
>>> 'compare' method may take CharSequence implementations such as String,
>>> StringBuilder and StringBuffer, making it possible to perform
>>> comparison
>>> among them. The previous example for example is equivalent to
>>> CharSequence.compare(aStringBuilder, anotherStringBuilder).
>>>
>>> Tests for java.base have been independent from each other. The new
>>> tests are
>>> therefore created to have no dependency on each other or sharing any
>>> code.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
>>> webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/
>>>
>>> Thanks,
>>> Joe

Reply via email to