Looks good. Jason ________________________________________ From: Joe Wang <huizhe.w...@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-boun...@openjdk.java.net> on behalf of Joe > Wang<huizhe.w...@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-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