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

[Jason Mehrens]

Looks good.

Jason
________________________________________
From: Joe Wang <huizhe.wang at oracle.com>
Sent: Monday, February 12, 2018 12:25 PM
To: Jason Mehrens
Cc: core-libs-dev
Subject: Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

Done. Thanks Jason!

Joe


On 2/9/18, 1:46 PM, Jason Mehrens wrote:
> 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