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