Please review - 6984084 - String repeat()

Mike Duigou mike.duigou at oracle.com
Wed Aug 15 12:35:30 PDT 2012


webrev::

- I use "duke" (which is a valid OpenJDK id used for admin activities) as my reviewed-by reviewer until I have real reviewers.

- my OpenJDK id is mduigou (your webrev is missing a "u").

StringJoiner::

- Why is a temporary introduced into add(CharSequence?)

- The test n > 0 is unnecessary as the loop condition will have the same effect.

- Why accept and ignore n <= 0 and not instead throw a IAE? (I always suspect that this will hide errors).

AbstractStringBuilder:: 

- behaviour for negative n is rather surprising to me. I'd rather just see an IAE.

- Rather than using append and n-1 you might want to use the "set value" phrasing. ie. 
"Set the value of this StringBuilder to be n concatenated copies of the current value."


String::

- behaviour for negative n is rather surprising to me. I'd rather just see an IAE.

- Rather than using append and n-1 you might want to use the "set value" phrasing. ie. 
"Return a String consisting of n concatenated copies of this String."


On Aug 15 2012, at 11:41 , Jim Gish wrote:

> Please review http://cr.openjdk.java.net/~jgish/6984084-StringRepeat/
> 
> This in response to an old RFE to add either a new constructor or a 
> method to repeat a string, such as "*".repeat(5) resutling in "*****"
> 
> There is nothing particularly lambda related in here other then the fact 
> that under the covers I'm using StringJoiner.
> 
> I'm happy with the name "repeat" but question the "addNcopies" method in 
> AbstractStringBuilder -- suggestions welcome.
> 
> Thanks,
>    Jim
> 
> -- Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304 
> Oracle Java Platform Group | Core Libraries Team 35 Network Drive 
> Burlington, MA 01803 jim.gish at oracle.com
> 



More information about the lambda-dev mailing list