garydgregory commented on code in PR #1644:
URL: https://github.com/apache/commons-lang/pull/1644#discussion_r3247707405
##########
src/main/java/org/apache/commons/lang3/StringUtils.java:
##########
@@ -6117,7 +6117,12 @@ public static String repeat(final String repeat, final
int count) {
if (inputLength == 1 && count <= PAD_LIMIT) {
return repeat(repeat.charAt(0), count);
}
- final int outputLength = inputLength * count;
+ final int outputLength;
+ try {
+ outputLength = Math.multiplyExact(inputLength, count);
+ } catch (final Exception e) {
+ throw new IllegalArgumentException("Count too large for input");
Review Comment:
Interesting suggestion, I think the JRE made a mistake here and didn't read
its own spec (see below).
`OutOfMemoryError` is an bad choice IMO because:
- We've not actually run out of memory, we are preempting execution, we
detected bad input that could be recoverable by an app. For example: ask the
user to pick a smaller file, a smaller this, a smaller that.
- The PR converts a runtime exception into another runtime exception.
- Your suggestion would convert a runtime exception into an error. Now an
app should catch `IllegalArgumentException` and `OutOfMemoryError` to handle
bad input sizes. It is unusual for apps to catch errors.
- How does an app detect the difference b/w actually running out of memory
and bad input?
- `OutOfMemoryError` says: "Thrown when the Java Virtual Machine cannot
allocate an object because it is out of memory, and no more memory could be
made available by the garbage collector."
That's definitely _not_ what happened here, or in `String.repeat(int).`
- `OutOfMemoryError` is a subclass of `VirtualMachineError` which says:
"Thrown to indicate that the Java Virtual Machine is broken or has run out of
resources necessary for it to continue operating."
Note the "for it to continue operating". If we throw OOME, we are saying the
JVM cannot continue operating.
The message here could be better: "The requested result is too large for a
String." Better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
