Good to have the data. Thank you Louis. Sent from my iPhone
> On Feb 15, 2018, at 4:52 PM, Louis Wasserman <lowas...@google.com> wrote: > > I don't think there's a case for demand to merit having a > repeat(CharSequence, int) at all. > > I did an analysis of usages of Guava's Strings.repeat on Google's codebase. > Users might be rolling their own implementations, too, but this should be a > very good proxy for demand. > > StringRepeat_SingleConstantChar = 4.475 K // strings with .length() == > 1 > StringRepeat_SingleConstantCodePoint = 28 // strings with > .codePointCount(...) == 1 > StringRepeat_MultiCodePointConstant = 1.156 K // constant strings neither > of the above > StringRepeat_CharSequenceToString = 2 // > Strings.repeat(CharSequence.toString(), n) > StringRepeat_NoneOfTheAbove = 248 > > Notably, it seems like basically nobody needs to repeat a CharSequence -- > definitely not enough demand to merit the awkwardness of e.g. Rope.repeat(n) > inheriting a repeat returning a String. > > Based on this data, I'd recommend providing one and only one method of this > type: String.repeat(int). There's no real advantage to a static repeat(char, > int) method when the overwhelming majority of these are constants: e.g. > compare SomeUtilClass.repeat('*', n) versus "*".repeat(n). > Character.toString(c).repeat(n) isn't a bad workaround if you don't have a > constant char. There also isn't much demand for dealing with the code point > case specially; the String.repeat(int) method seems like it'd handle that > just fine. > >> On Thu, Feb 15, 2018 at 11:44 AM Jim Laskey <james.las...@oracle.com> wrote: >> >> >> > On Feb 15, 2018, at 3:36 PM, Ivan Gerasimov <ivan.gerasi...@oracle.com> >> > wrote: >> > >> > Hello! >> > >> > The link with the webrev returned 404, but I could find it at this >> > location: http://cr.openjdk.java.net/~jlaskey/8197594/webrev-00/ >> > >> > A few minor comments: >> > >> > 1) >> > >> > This check: >> > >> > 2992 final long limit = (long)count * 2L; >> > 2993 if ((long)Integer.MAX_VALUE < limit) { >> > >> > can be possibly simplified as >> > if (count > Integer.MAX_VALUE - count) { >> >> Good. >> >> > >> > 2) >> > Should String repeat(final int codepoint, final int count) be optimized >> > for codepoints that can be represented with a single char? >> > >> > E.g. like this: >> > >> > public static String repeat(final int codepoint, final int count) { >> > return Character.isBmpCodePoint(codepoint)) >> > ? repeat((char) codepoint, count) >> > : (new String(Character.toChars(codepoint))).repeat(count); >> > } >> >> Yes, avoid array allocation. >> >> > >> > 3) >> > Using long arithmetic can possibly be avoided in the common path of >> > repeat(final int count): >> > >> > E.g. like this: >> > >> > if (count < 0) { >> > throw new IllegalArgumentException("count is negative, " + >> > count); >> > } else if (count == 1) { >> > return this; >> > } else if (count == 0) { >> > return ""; >> > } >> > final int len = value.length; >> > if (Integer.MAX_VALUE / count < len) { >> > throw new IllegalArgumentException( >> > "Resulting string exceeds maximum string length: " + >> > ((long)len * (long)count)); >> > } >> > final int limit = count * len; >> >> Good. >> >> Thank you. >> >> > >> > With kind regards, >> > Ivan >> > >> > On 2/15/18 9:20 AM, Jim Laskey wrote: >> >> This is a pre-CSR code review [1] for String repeat methods (Enhancement). >> >> >> >> The proposal is to introduce four new methods; >> >> >> >> 1. public String repeat(final int count) >> >> 2. public static String repeat(final char ch, final int count) >> >> 3. public static String repeat(final int codepoint, final int count) >> >> 4. public static String repeat(final CharSequence seq, final int count) >> >> >> >> For the sake of transparency, only 1 is necessary, 2-4 are convenience >> >> methods. >> >> In the case of 2, “*”.repeat(10) performs as well as String.repeat(‘*’, >> >> 10). >> >> 3 and 4 convert to String before calling 1. >> >> >> >> Performance runs with jmh (results as comment in [2]) show that these >> >> methods are significantly faster that StringBuilder equivalents. >> >> - fewer memory allocations >> >> - fewer char to byte array conversions >> >> - faster pyramid replication vs O(N) copying >> >> >> >> I left StringBuilder out of scope. It falls under the category of >> >> Appendables#append with repeat. A much bigger project. >> >> >> >> All comments welcome. Especially around the need for convenience >> >> methods, the JavaDoc content and expanding the tests. >> >> >> >> — Jim >> >> >> >> [1] webrev: http://cr.openjdk.java.net//oj/home/jlaskey/8197594/webrev-00 >> >> [2] jbs: https://bugs.openjdk.java.net/browse/JDK-8197594 >> >> >> >> >> > >> > -- >> > With kind regards, >> > Ivan Gerasimov >> > >>