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
