RFR: 8197594 - String and character repeat

[Ivan Gerasimov]

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) {

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);
}

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;

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