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

2021-06-25 Thread Joe Darcy

On 6/21/2021 2:02 PM, Paul Sandoz wrote:

On Mon, 21 Jun 2021 05:17:09 GMT, Yi Yang  wrote:


After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
the whole JDK codebase.

Yi Yang has updated the pull request incrementally with one additional commit 
since the last revision:

   more replacement 2

All the updates to the check* methods look ok (requires some careful looking!).

I cannot recall what others said about the change in exception messages. 
@jddarcy any advice here?


Generally, the JDK does not have the text of exception message as a 
supported interface meant to be relied on by users. This doesn't stop 
developers from occasionally parsing those messages, but that is usually 
a sign of a missing API which we try to rectify more directly.


HTH,

-Joe



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

2021-06-21 Thread Yi Yang
On Mon, 21 Jun 2021 20:49:56 GMT, Paul Sandoz  wrote:

>> Yi Yang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   more replacement 2
>
> src/java.base/share/classes/jdk/internal/util/Preconditions.java line 78:
> 
>> 76: = Preconditions.outOfBoundsExceptionFormatter(new 
>> StringIndexOutOfBoundsExceptionProducer());
>> 77: 
>> 78: public static final BiFunction, 
>> StringIndexOutOfBoundsException> AIOOBE_FORMATTER
> 
> Using incorrect exception type. Suggest you embed as inner class rather than 
> separate declaration, since they are only used in one place.

Fixed.

FYI: Current exception message looks like this:

Exception in thread "main" java.lang.StringIndexOutOfBoundsException: Range [3, 
1) out of bounds for length 6
at 
CheckIndex$StringIndexOutOfBoundsExceptionProducer.apply(CheckIndex.java:77)
at 
CheckIndex$StringIndexOutOfBoundsExceptionProducer.apply(CheckIndex.java:72)
at 
java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:159)
at 
java.base/jdk.internal.util.Preconditions$1.apply(Preconditions.java:156)
at 
java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:62)
at 
java.base/jdk.internal.util.Preconditions.outOfBoundsCheckFromToIndex(Preconditions.java:76)
at 
java.base/jdk.internal.util.Preconditions.checkFromToIndex(Preconditions.java:295)
at CheckIndex.main(CheckIndex.java:110)

I think now it expresses more exception information than before(and more 
consistent).

-

PR: https://git.openjdk.java.net/jdk/pull/4507


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

2021-06-21 Thread Paul Sandoz
On Mon, 21 Jun 2021 05:17:09 GMT, Yi Yang  wrote:

>> After JDK-8265518(#3615), it's possible to replace all variants of 
>> checkIndex by 
>> Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
>> the whole JDK codebase.
>
> Yi Yang has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   more replacement 2

All the updates to the check* methods look ok (requires some careful looking!).

I cannot recall what others said about the change in exception messages. 
@jddarcy any advice here?

src/java.base/share/classes/jdk/internal/util/Preconditions.java line 78:

> 76: = Preconditions.outOfBoundsExceptionFormatter(new 
> StringIndexOutOfBoundsExceptionProducer());
> 77: 
> 78: public static final BiFunction, 
> StringIndexOutOfBoundsException> AIOOBE_FORMATTER

Using incorrect exception type. Suggest you embed as inner class rather than 
separate declaration, since they are only used in one place.

-

PR: https://git.openjdk.java.net/jdk/pull/4507


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

2021-06-20 Thread Yi Yang
> After JDK-8265518(#3615), it's possible to replace all variants of checkIndex 
> by Objects.checkIndex/Objects.checkFromToIndex/Objects.checkFromIndexSize in 
> the whole JDK codebase.

Yi Yang has updated the pull request incrementally with one additional commit 
since the last revision:

  more replacement 2

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4507/files
  - new: https://git.openjdk.java.net/jdk/pull/4507/files/c8b2106e..3a8875ec

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=4507=05
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4507=04-05

  Stats: 32 lines in 2 files changed: 1 ins; 20 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4507.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4507/head:pull/4507

PR: https://git.openjdk.java.net/jdk/pull/4507