Hi Stuart,
Thanks for the apiNote! Joe re-approved the CSR with the added apiNote.
For a test that compares Latin1 vs UTF16 coders, I added tests to the
compact string's CompareTo test[1], basically running the original test
for String with StringBuilder/Buffer. A build with jdk_core tests passed.
Here's the updated webrev:
JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/
[1]
http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/test/jdk/java/lang/String/CompactString/CompareTo.java.sdiff.html
Best,
Joe
On 2/14/18, 5:09 PM, Stuart Marks wrote:
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-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
