RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

[Jason Mehrens]

Joe,
In CharSequence, does it make sense to cache the result of the length calculation?
As in change:
====
for (int i = 0; i < Math.min(cs1.length(), cs2.length()); i++) {
====
for (int i = 0, len = Math.min(cs1.length(), cs2.length()); i < len; i++) {
====

Jason
________________________________________
From: core-libs-dev <core-libs-dev-bounces at openjdk.java.net> on behalf of Joe Wang <huizhe.wang at oracle.com>
Sent: Thursday, February 8, 2018 6:47 PM
To: core-libs-dev
Subject: Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

Hi all,

The CSR for the enhancement is now approved. Thanks Joe!

The webrev has been updated accordingly. Please let me know if you have
any further comment on the implementation.
JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Thanks,
Joe

On 2/2/2018 12:46 PM, Joe Wang wrote:
> Thanks Jason. Will update that accordingly.
>
> Best,
> Joe
>
> On 2/2/2018 11:22 AM, Jason Mehrens wrote:
>> 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-bounces at openjdk.java.net> on
>> behalf of Joe Wang <huizhe.wang at 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.wang at 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
>