On Thu, 23 Feb 2023 15:52:15 GMT, Roger Riggs <rri...@openjdk.org> wrote:
>> Add the ability to repeatedly append char and CharSequence data to
>> StringBuilder/StringBuffer.
>
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1839:
>
>> 1837: * @throws StringIndexOutOfBoundsException if the result
>> overflows the buffer
>> 1838: */
>> 1839: public AbstractStringBuilder repeat(char c, int count) {
>
> The shadowing of the count field is a hidden maintenance trap.
I know but Stuart requested it.
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1867:
>
>> 1865:
>> 1866: private AbstractStringBuilder repeatNull(int count) {
>> 1867: if (count < 0) {
>
> This could be implemented as `repeat("null", count)`. Its not likely to be
> performance sensitive and be easier to maintain.
Just being consistent with existing code.
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1869:
>
>> 1867: if (count < 0) {
>> 1868: throw new IllegalArgumentException("count is less than
>> zero: " + count);
>> 1869: } else if (count == 0) {
>
> Inconsistent `if ... else if ...` structure compared to method above.
Noted. Will fix.
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1879:
>
>> 1877: throw new OutOfMemoryError("Required length exceeds
>> implementation limit");
>> 1878: }
>> 1879: int limit = count * length;
>
> The meaning of limit should be consistent across uses. Above it is an index
> into the buffer; here it is an offset.
Not seeing it.
> src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1904:
>
>> 1902: public AbstractStringBuilder repeat(CharSequence cs, int count) {
>> 1903: if (cs == null) {
>> 1904: return repeatNull(count);
>
> Or just replace `cs = "null";` and fall through to the rest.
Same.
-------------
PR: https://git.openjdk.org/jdk/pull/12728
