Hopefully some day we would have fixed: Bug 6373386 <http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6373386> - Method chaining for instance methods that return void That would prevent us from such bloated sub classes. Hopefully such "workarounds" would not force later incompatibilities -Ulf Am 23.08.2012 02:35, schrieb David Holmes:

On 22/08/2012 11:52 AM, David Holmes wrote:

...

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.

I missed the covariant change to the return type from AbstractStringBuilder to StringBuilder.

I also suggested in email that the above could simply be:

return super.append(n, cs);

but that would also need a cast to StringBuilder. So what Jim has is fine.

David -----

FWIW, the few times I "needed" a repeat operation it's always been with a char, not a string. I think a common use case for this to create some layout in the printed string, such as adding a separator (e.g. "*********", "---------", etc) or adding white space padding (could also be something like "..........") to align output - both are just chars. Sent from my phone On Aug 22, 2012 5:47 AM, "Alan Bateman" <Alan.Bateman@oracle.com> wrote:

On 21/08/2012 21:45, Jim Gish wrote:

...

Please review http://cr.openjdk.java.net/~**jgish/6984084-jdk8-** StringRepeat/<http://cr.openjdk.java.net/~jgish/6984084-jdk8-StringRepeat/><; http://cr.openjdk.java.net/%**7Ejgish/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.

On the surface then String s = "foo".repeat(5); seems nice. I just wonder whether we can get any data on usage of similar methods in commonly used libraries. Some languages such as Python have a built-ins for this, and I think C# has a ctor, just not clear (to me anyway) how often the latter or equivalent it used. Folks here might be able to jump in to indicate how often they've needed to do this and whether the common-case is a repeated char rather than String (or CharSequence). Folks here might also have ideas on getting some usage data in other libraries or languages.

I'm less sure about StringBuilder.append(int,**CharSequence) as sb.append(5,cs) is not much less than "for (int i=0; i<5; i++) sb.append(cs);". I also wonder whether it may be more common to want to appear a number of chars, sb.append(5, '-') for example.

Anyway, on the javadoc then AbstractStringBuilder.append suggests special hanging for n==1 but I think that can be dropped the javadoc.

On the implementation then it would be good to make the style consistent with the existing code (4 spaces for indent, etc.). Otherwise it seems okay.

I assume the update to test/Makefile is just for your own local usage and that you aren't planning to include this (the jdk_lang target already runs the tests in test/java/lang/**).

In Repeat.java (same thing in AppendIntCharSequence.java) then I think the -1 test can be simplified to:

try { "c".repeat(-1); throw new RuntimeException(...); } catch (IllegalArgumenetException expected) { }

We use GPL on the tests so you need to check AppendIntCharSequence.java.

On test coverage then it looks like StringBuffer is missed. Also the assertion that appending null will append "null".

-Alan.