On Thu, 23 Feb 2023 13:13:43 GMT, Jim Laskey <jlas...@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.
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.
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.
src/java.base/share/classes/java/lang/AbstractStringBuilder.java line 1876:
> 1874: int length = this.count - offset;
> 1875: int valueLength = length << coder;
> 1876: if ((Integer.MAX_VALUE - offset) / count < valueLength) {
Can this check be done with multiply instead of divide?
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.
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.
-------------
PR: https://git.openjdk.org/jdk/pull/12728
