RFR: 8268698: Use Objects.check{Index, FromToIndex, FromIndexSize} where possible [v2]

[Yi Yang]

On Thu, 17 Jun 2021 15:45:47 GMT, Paul Sandoz <psandoz at 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