RFR: 8265137: java.util.Random suddenly has new public methods nowhere documented [v6]

[Stuart Marks]

On Wed, 21 Apr 2021 13:13:16 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> Move makeXXXSpilterator from public (@hidden) to protected. No API ch
>
> Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
> 
>  - Merge branch 'master' into 8265137
>  - Remove @hidden
>  - Correct the hierarchy of Random
>  - Remove extraneous references to makeXXXSpliterator
>  - Move makeXXXSpliterator methods to RandomSupport
>  - change static final from 'proxy' to 'PROXY'
>  - Make makeXXXSpliterator final
>  - Move makeXXXSpilterator from public (@hidden) to protected. No API change.

Overall looks good.

AbstractSpliteratorGenerator now is a bit weird because it's a utility class that has static methods, _and_ it's an abstract superclass that has instance methods that mostly just call the static methods. Seems to me that additional cleanup is possible. It might not be worth it right now, since the main point of this PR -- to remove "leakage" of these helper methods into the public API -- has been achieved.

The RandomXXXSpliterator nested classes could also stand some scrutiny. The constructors consume a RandomGenerator, which is stored in an instance variable. Various comments still refer to this as an AbstractSpliteratorGenerator. (See line 961 for example; can't comment directly because it's not part of this commit.)

But it's not clear to me that the RandomXXXSpliterator classes really need a full RandomGenerator. For example, as far as I can see, RandomIntSpliterator _only_ needs `nextInt` to generate int values; therefore it could be `IntSupplier`. Similar for long and double. (I'm applying the Interface Segregation Principle here.) But again this is probably a cleanup for another day.

src/java.base/share/classes/jdk/internal/util/random/RandomSupport.java line 1433:

> 1431:         private AbstractSpliteratorGenerator() {
> 1432:         }
> 1433: 

This comment is wrong now since there are instances of this class (really, subclasses). The constructor should probably simply be removed.

-------------

Marked as reviewed by smarks (Reviewer).

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