On Thu, 17 Jun 2021 15:45:47 GMT, Paul Sandoz <psan...@openjdk.org> wrote:
>> src/java.base/share/classes/java/util/Base64.java line 934: >> >>> 932: if (closed) >>> 933: throw new IOException("Stream is closed"); >>> 934: Preconditions.checkFromIndexSize(len, off, b.length, (xa, >>> xb) -> new ArrayIndexOutOfBoundsException()); >> >> You might want to split this really long line too, to avoid inconsistent >> line length in this source file. > > I would suggest factoring out `(xa, xb) -> new > ArrayIndexOutOfBoundsException()` as a reusable component on `Preconditions`, > and maybe even supplying an exception message (since it is rather useful, and > way better than no message). > > See the usages of `Preconditions.outOfBoundsExceptionFormatter` (where there > just happens to be many repeated cases of supplying AIOOBE with a message, > that could also be consolidated, separate fix perhaps). Ok, I've replaced Preconditions.checkFromIndexSize(len, off, b.length, (xa, xb) -> new ArrayIndexOutOfBoundsException()); with Preconditions.checkFromIndexSize(len, off, b.length, Preconditions.outOfBoundsExceptionFormatter(ArrayIndexOutOfBoundsException::new)); ---- I am curious about the motivations of current APIs: public static <X extends RuntimeException> int checkIndex(int index, int length, BiFunction<String, List<Number>, X> oobef) { if (index < 0 || index >= length) throw outOfBoundsCheckIndex(oobef, index, length); return index; } Are they over-engineered? When I checked all checkIndex-like patterns in the whole jdk codebase, I found that in most cases, providing an API that accepts custom exception should be enough for scalability: int checkIndex(int index, int length, IndexOutOfBoundException iooe) { if (index < 0 || index >= length) throw iooe; return index; } In addition to the ease-of-use problem, there is another problem with the current API design. Some methods in j.l.String are good replacement candidates for Preconditions.check{Index,...}: https://github.com/openjdk/jdk/blob/a051e735cda0d5ee5cb6ce0738aa549a7319a28c/src/java.base/share/classes/java/lang/String.java#L4558-L4604 But we **can not** do such replacement after my practice. The third parameter of Preconditions.checkIndex is a BiFunction, which can be passed a lambda as its argument. The generated lambda method exercises many other codes like MethodHandles, j.l.Package, etc, eventually it called j.l.String.checkIndex itself, so if we replace j.l.String.checkIndex with `Preconditions.checkIndex(a,b,(x1,x2)->....)`, a StackOverflowError would occur at VM startup. If there is an API that accepts user-defined exceptions, I think we can apply Preconditions in more places. ------------- PR: https://git.openjdk.java.net/jdk/pull/4507