Please Review: 6984084 (str) n times repetition of character constructor for java.lang.String (original) (raw)
Vitaly Davidovich vitalyd at gmail.com
Wed Aug 22 12:34:46 UTC 2012
- Previous message: Please Review: 6984084 (str) n times repetition of character constructor for java.lang.String
- Next message: Please Review: 6984084 (str) n times repetition of character constructor for java.lang.String
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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 at 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 jdklang 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.
- Previous message: Please Review: 6984084 (str) n times repetition of character constructor for java.lang.String
- Next message: Please Review: 6984084 (str) n times repetition of character constructor for java.lang.String
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]