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

[Stuart Marks]

Hi Joe,

Overall, looks good.

Are there any tests of AbstractStringBuilder.compareTo() that exercise 
comparisons of the Latin1 vs UTF16 coders?

A couple people have raised the issue of the SB's implementing Comparable but 
not overriding equals(). This is unusual but well-defined. I do think it 
deserves the "inconsistent with equals" treatment. Something like the following 
should be added to both SB's:

==========
@apiNote
{@code StringBuilder} implements {@code Comparable} but does not override {@link 
Object#equals equals}. Thus, the natural ordering of {@code StringBuilder} is 
inconsistent with equals. Care should be exercised if {@code StringBuilder} 
objects are used as keys in a {@code SortedMap} or elements in a {@code 
SortedSet}. See {@link Comparable}, {@link java.util.SortedMap SortedMap}, or 
{@link java.util.SortedSet SortedSet} for more information.
==========

Respectively for StringBuffer. (Adjust markup to taste.)

Thanks,

s'marks




On 2/8/18 4:47 PM, Joe Wang wrote:
> 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
>>
>