Please Review: 6984084 (str) n times repetition of character constructor for java.lang.String

David Holmes david.holmes at oracle.com
Wed Aug 22 01:52:12 UTC 2012


Hi Jim,

On 22/08/2012 6:45 AM, Jim Gish wrote:
> Please review
> http://cr.openjdk.java.net/~jgish/6984084-jdk8-StringRepeat/
> <http://cr.openjdk.java.net/%7Ejgish/6984084-jdk8-StringRepeat/>
>
> This started in lambda, making changes to both StringJoiner and String.
> However, the dependence of String.repeat() on StringJoiner has been
> removed and the resulting non-lambda classes moved here for review.
> There will be a separate change and review for the StringJoiner changes
> in lamda-dev.

AbstractStringBuilder.java:

"If {@code n == 0}, then adds the empty string."

It doesn't - if n == 0 nothing happens (which seems the right thing to me).

"If @{code cs} is {@code null}, then adds {@code "null"} {@code n} times."

That seems questionable semantics to me. What is the motivation for this?

---

String.java

Does repeat() really carry its own weight? The 0 and 1 cases are 
uninteresting and for anything else StringBuilder can be used directly.

---

StringBuilder.java

       /**
+      * @throws IllegalArgumentException if n < 0  {@inheritDoc}

This is wrong as you've already written the inherited doc "if n < 0".

+      * @since 1.8
+      */
+     public StringBuilder append(int n, CharSequence cs) {
+         super.append(n, cs);
+         return this;
+     }

But why are you overriding this in the first place ??? There seems to be 
no change in functionality or specification.

David
-----

> Thanks,
> Jim
>



More information about the core-libs-dev mailing list