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

[Roger Riggs]

+1; looks good

On 2/27/2018 3:15 PM, Xueming Shen wrote:

+1

On 2/27/18, 11:37 AM, Joe Wang wrote:

Hi Sherman and all,

Thanks for the further reviews!

Here's the latest webrev with boundary checks in 
StringLatin1/StringUTF16:


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Best,
Joe

On 2/22/2018 6:07 PM, Joe Wang wrote:



On 2/22/18, 12:51 PM, Xueming Shen wrote:

On 2/22/18, 12:04 PM, Joe Wang wrote:

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check 
would be almost immediately done against the field itself. Are you 
worrying about the count field may be out of sync with the byte 
array? I would think that's unlikely to happen. Whether it's 
StringBuilder or StringBuffer, it's not advisable/practical to use 
in multiple threads. In a valid usage, the count is always 
consistent with the byte array.




Hi Joe,

It might not be a "valid usage" but it did happen and when it 
happens it might just crash the
vm without those boundary checks. It's especially true for those 
intrinsics methods with explicit
comments "intrinsic performs no bounds checks". In this case, the 
StringUTF16.getChar() is being
called in new public method StringUTF16.compareTo(byte[], byte[], 
int, int) without appropriate
boundary check. In the "old" code the "index" is guaranteed to be 
within [0, len) in
StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case 
for such scenario can be found in

JDK-8158168 [1], for example.


Thanks for the pointer! The email thread helps a lot. I've updated 
the webrev with a boundary check in ASB (AbstractStringBuilder line 
106, 107), and then a note to StringUTF16.compareTo (StringUTF16 
line 280). Hopefully this is sufficient. Didn't want to add any 
check in StringUTF16 since that may affect the original two-arg method.


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

-Joe



-Sherman


[1] https://bugs.openjdk.java.net/browse/JDK-8158168

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

[Xueming Shen]

+1

On 2/27/18, 11:37 AM, Joe Wang wrote:

Hi Sherman and all,

Thanks for the further reviews!

Here's the latest webrev with boundary checks in 
StringLatin1/StringUTF16:


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Best,
Joe

On 2/22/2018 6:07 PM, Joe Wang wrote:



On 2/22/18, 12:51 PM, Xueming Shen wrote:

On 2/22/18, 12:04 PM, Joe Wang wrote:

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check 
would be almost immediately done against the field itself. Are you 
worrying about the count field may be out of sync with the byte 
array? I would think that's unlikely to happen. Whether it's 
StringBuilder or StringBuffer, it's not advisable/practical to use 
in multiple threads. In a valid usage, the count is always 
consistent with the byte array.




Hi Joe,

It might not be a "valid usage" but it did happen and when it 
happens it might just crash the
vm without those boundary checks. It's especially true for those 
intrinsics methods with explicit
comments "intrinsic performs no bounds checks". In this case, the 
StringUTF16.getChar() is being
called in new public method StringUTF16.compareTo(byte[], byte[], 
int, int) without appropriate
boundary check. In the "old" code the "index" is guaranteed to be 
within [0, len) in
StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case for 
such scenario can be found in

JDK-8158168 [1], for example.


Thanks for the pointer! The email thread helps a lot. I've updated 
the webrev with a boundary check in ASB (AbstractStringBuilder line 
106, 107), and then a note to StringUTF16.compareTo (StringUTF16 line 
280). Hopefully this is sufficient. Didn't want to add any check in 
StringUTF16 since that may affect the original two-arg method.


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

-Joe



-Sherman


[1] https://bugs.openjdk.java.net/browse/JDK-8158168

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

[Joe Wang]

Hi Sherman and all,

Thanks for the further reviews!

Here's the latest webrev with boundary checks in StringLatin1/StringUTF16:

JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

Best,
Joe

On 2/22/2018 6:07 PM, Joe Wang wrote:



On 2/22/18, 12:51 PM, Xueming Shen wrote:

On 2/22/18, 12:04 PM, Joe Wang wrote:

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check would 
be almost immediately done against the field itself. Are you 
worrying about the count field may be out of sync with the byte 
array? I would think that's unlikely to happen. Whether it's 
StringBuilder or StringBuffer, it's not advisable/practical to use 
in multiple threads. In a valid usage, the count is always 
consistent with the byte array.




Hi Joe,

It might not be a "valid usage" but it did happen and when it happens 
it might just crash the
vm without those boundary checks. It's especially true for those 
intrinsics methods with explicit
comments "intrinsic performs no bounds checks". In this case, the 
StringUTF16.getChar() is being
called in new public method StringUTF16.compareTo(byte[], byte[], 
int, int) without appropriate
boundary check. In the "old" code the "index" is guaranteed to be 
within [0, len) in
StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case for 
such scenario can be found in

JDK-8158168 [1], for example.


Thanks for the pointer! The email thread helps a lot. I've updated the 
webrev with a boundary check in ASB (AbstractStringBuilder line 106, 
107), and then a note to StringUTF16.compareTo (StringUTF16 line 280). 
Hopefully this is sufficient. Didn't want to add any check in 
StringUTF16 since that may affect the original two-arg method.


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

-Joe



-Sherman


[1] https://bugs.openjdk.java.net/browse/JDK-8158168

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

[Joe Wang]

On 2/22/18, 12:51 PM, Xueming Shen wrote:

On 2/22/18, 12:04 PM, Joe Wang wrote:

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check would 
be almost immediately done against the field itself. Are you worrying 
about the count field may be out of sync with the byte array? I would 
think that's unlikely to happen. Whether it's StringBuilder or 
StringBuffer, it's not advisable/practical to use in multiple 
threads. In a valid usage, the count is always consistent with the 
byte array.




Hi Joe,

It might not be a "valid usage" but it did happen and when it happens 
it might just crash the
vm without those boundary checks. It's especially true for those 
intrinsics methods with explicit
comments "intrinsic performs no bounds checks". In this case, the 
StringUTF16.getChar() is being
called in new public method StringUTF16.compareTo(byte[], byte[], int, 
int) without appropriate
boundary check. In the "old" code the "index" is guaranteed to be 
within [0, len) in
StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case for 
such scenario can be found in

JDK-8158168 [1], for example.


Thanks for the pointer! The email thread helps a lot. I've updated the 
webrev with a boundary check in ASB (AbstractStringBuilder line 106, 
107), and then a note to StringUTF16.compareTo (StringUTF16 line 280). 
Hopefully this is sufficient. Didn't want to add any check in 
StringUTF16 since that may affect the original two-arg method.


JBS: https://bugs.openjdk.java.net/browse/JDK-8137326
webrev: http://cr.openjdk.java.net/~joehw/jdk11/8137326/webrev/

-Joe



-Sherman


[1] https://bugs.openjdk.java.net/browse/JDK-8158168

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

[Xueming Shen]

On 2/22/18, 12:04 PM, Joe Wang wrote:

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check would 
be almost immediately done against the field itself. Are you worrying 
about the count field may be out of sync with the byte array? I would 
think that's unlikely to happen. Whether it's StringBuilder or 
StringBuffer, it's not advisable/practical to use in multiple threads. 
In a valid usage, the count is always consistent with the byte array.




Hi Joe,

It might not be a "valid usage" but it did happen and when it happens it 
might just crash the
vm without those boundary checks. It's especially true for those 
intrinsics methods with explicit
comments "intrinsic performs no bounds checks". In this case, the 
StringUTF16.getChar() is being
called in new public method StringUTF16.compareTo(byte[], byte[], int, 
int) without appropriate
boundary check. In the "old" code the "index" is guaranteed to be within 
[0, len) in
StringUTF16.compareTo(byte[], byte[]), so it's safe. A real case for 
such scenario can be found in

JDK-8158168 [1], for example.

-Sherman


[1] https://bugs.openjdk.java.net/browse/JDK-8158168

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

[Joe Wang]

Hi Sherman,

Thanks for reviewing the change.

Taking a local copy of the count field, but the boundary check would be 
almost immediately done against the field itself. Are you worrying about 
the count field may be out of sync with the byte array? I would think 
that's unlikely to happen. Whether it's StringBuilder or StringBuffer, 
it's not advisable/practical to use in multiple threads. In a valid 
usage, the count is always consistent with the byte array.


Best,
Joe

On 2/21/2018 11:59 AM, Xueming Shen wrote:

Joe,

AbstractStringBuilder is a mutable class. You might need to take a 
local copy
and do the explicit boundary check before going into the 
StringLatin1/UTF16,
especially StringUTF16. Some intrinsics, UTF16.getChar(byte[], int) 
for example,

assume the caller performs bounds check (for performance reason).

-Sherman

On 2/21/18, 11:25 AM, Joe Wang wrote:

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 

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

[Joe Wang]

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 

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

[Xueming Shen]

Joe,

AbstractStringBuilder is a mutable class. You might need to take a local 
copy

and do the explicit boundary check before going into the StringLatin1/UTF16,
especially StringUTF16. Some intrinsics, UTF16.getChar(byte[], int) for 
example,

assume the caller performs bounds check (for performance reason).

-Sherman

On 2/21/18, 11:25 AM, Joe Wang wrote:

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 t

Re: 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-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

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

[Jason Mehrens]

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

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

[Joe Wang]

On 2/9/18, 1:38 PM, Roger Riggs wrote:

Hi Joe,

Looking good, but a few comments:

AbstractStringBuilder: 111  the coder() method should be private and 
since there is only a few uses

  the function could be inlined.


Indeed, the coder() method was added along with the new method. The 
coder field was referenced directly in all existing uses. I've removed 
the coder() method, and instead refer to the coder field directly.


StringBuffer:192: extra leading space before "}"


Fixed.

Thanks,
Joe



Thanks, Roger

On 2/8/2018 7: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

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

[Joe Wang]

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

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

[Jason Mehrens]

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
>

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

[Roger Riggs]

Hi Joe,

Looking good, but a few comments:

AbstractStringBuilder: 111  the coder() method should be private and 
since there is only a few uses

  the function could be inlined.

StringBuffer:192: extra leading space before "}"

Thanks, Roger

On 2/8/2018 7: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

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

[Joe Wang]

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

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

[Joe Wang]

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

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

[Jason Mehrens]

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

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

[Joe Wang]

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  
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

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

[Joe Wang]

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  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

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

[Tagir Valeev]

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  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

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

[huizhe wang]

Thanks Sherman.  Found it now.  Interesting method. What was the 
motivation? Does any class implement CharSequence in 310?


Best,
Joe

On 1/26/2018 2:12 PM, Xueming Shen wrote:


java.time.format.DateTimeParseContext.subSequenceEquals

On 1/26/18, 1:48 PM, huizhe wang wrote:

Thanks Stephen for the note.

I downloaded the api [1], but don't seem to see a class 
"DateTimeParseContext". Do you have a pointer to the current webrev? 
What does it do, and what would be the implication with regards to 
the CharSequence compare method?


[1] https://jcp.org/en/jsr/detail?id=310

Best,
Joe

On 1/26/2018 7:01 AM, Stephen Colebourne wrote:

Just to note that JSR-310 had to add a CharSequence comparison method.
Not the same as this one, but a data point, and perhaps a target to
become public in a future webrev. See
DateTimeParseContext.subSequenceEquals().

Stephen


On 26 January 2018 at 03:00, Joe Wang  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

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

[Xueming Shen]

java.time.format.DateTimeParseContext.subSequenceEquals

On 1/26/18, 1:48 PM, huizhe wang wrote:

Thanks Stephen for the note.

I downloaded the api [1], but don't seem to see a class 
"DateTimeParseContext". Do you have a pointer to the current webrev? 
What does it do, and what would be the implication with regards to the 
CharSequence compare method?


[1] https://jcp.org/en/jsr/detail?id=310

Best,
Joe

On 1/26/2018 7:01 AM, Stephen Colebourne wrote:

Just to note that JSR-310 had to add a CharSequence comparison method.
Not the same as this one, but a data point, and perhaps a target to
become public in a future webrev. See
DateTimeParseContext.subSequenceEquals().

Stephen


On 26 January 2018 at 03:00, Joe Wang  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

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

[huizhe wang]

Thanks Stephen for the note.

I downloaded the api [1], but don't seem to see a class 
"DateTimeParseContext". Do you have a pointer to the current webrev? 
What does it do, and what would be the implication with regards to the 
CharSequence compare method?


[1] https://jcp.org/en/jsr/detail?id=310

Best,
Joe

On 1/26/2018 7:01 AM, Stephen Colebourne wrote:

Just to note that JSR-310 had to add a CharSequence comparison method.
Not the same as this one, but a data point, and perhaps a target to
become public in a future webrev. See
DateTimeParseContext.subSequenceEquals().

Stephen


On 26 January 2018 at 03:00, Joe Wang  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

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

[huizhe wang]

Thanks for the comments, Jason.

The topic about equals/hashcode had been discussed. They are currently 
inherited, and by definition of CharSequence, equality is generally 
undefined. One could argue for it to be a lexicographic comparison, but 
that means changing the current specification, potentially breaking 
compatibility. Note that we made subtle wording change to the general 
description of CharSequence.


For #2, I'll add a clarification in the spec similar to those that deal 
with appending a CharSequence or StringBuffer, that the method does not 
synchronize on the source.  A better concurrent lock mechanism could 
have been devised for StringBuffer, but it'd be a fruitless effort to 
attempt to change its poor state. Given the nature of these classes, in 
almost all cases, StringBuilder is recommended over StringBuffer. So our 
effort here is to maintain a consistency between the classes.


#3, Concurrency would be better handled outside of the compare method 
(same as the above).


#4, sounds good. I'll look into the modification and probably add some 
more test cases.


Best,
Joe

On 1/26/2018 6:23 AM, Jason Mehrens wrote:

Joe,

1. Seems odd that StringBuilder and StringBuffer are comparable but don't have 
an equals/hashcode implementation.
Seems like if you are to stick with that you'll have to add the standard "natural 
ordering is inconsistent with equals" verbiage.
2. For StringBuffer compare, won't you have to either lock the 'another' object 
too so it is not mutated or perform some
sort of copy before compare? Neither sound like a good option.
3. Does CharSequence.compare need to specify that IndexOutOfBoundsException may 
be thrown under concurrent modification?
4. For CharSequence.compare, instead of creating a special case for string 
would it make sense to do:


if (cs1.getClass() == cs2.getClass() && cs1 instanceof Comparable) {
 return ((Comparable) cs1).compareTo(cs2);
  }


Given #1 and #2 maybe StringBuilder and StringBuffer shouldn't implement 
comparable and just rely on users either calling
  'sb1.toString().compare(sb2.toString())' or 'CharSequence.compare(sb1, sb2)'.

Jason

From: core-libs-dev  on behalf of Joe Wang 

Sent: Thursday, January 25, 2018 9:00 PM
To: core-libs-dev
Subject: RFR (JDK11) 8137326: Methods for comparing CharSequence, 
StringBuilder, and StringBuffer

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

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

[Stephen Colebourne]

Just to note that JSR-310 had to add a CharSequence comparison method.
Not the same as this one, but a data point, and perhaps a target to
become public in a future webrev. See
DateTimeParseContext.subSequenceEquals().

Stephen


On 26 January 2018 at 03:00, Joe Wang  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

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

[Jason Mehrens]

Joe,

1. Seems odd that StringBuilder and StringBuffer are comparable but don't have 
an equals/hashcode implementation.
Seems like if you are to stick with that you'll have to add the standard 
"natural ordering is inconsistent with equals" verbiage.
2. For StringBuffer compare, won't you have to either lock the 'another' object 
too so it is not mutated or perform some
sort of copy before compare? Neither sound like a good option.
3. Does CharSequence.compare need to specify that IndexOutOfBoundsException may 
be thrown under concurrent modification?
4. For CharSequence.compare, instead of creating a special case for string 
would it make sense to do:


if (cs1.getClass() == cs2.getClass() && cs1 instanceof Comparable) {
return ((Comparable) cs1).compareTo(cs2);
 }


Given #1 and #2 maybe StringBuilder and StringBuffer shouldn't implement 
comparable and just rely on users either calling
 'sb1.toString().compare(sb2.toString())' or 'CharSequence.compare(sb1, sb2)'.

Jason

From: core-libs-dev  on behalf of Joe 
Wang 
Sent: Thursday, January 25, 2018 9:00 PM
To: core-libs-dev
Subject: RFR (JDK11) 8137326: Methods for comparing CharSequence, 
StringBuilder, and StringBuffer

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