RFR: 8248862: Implement Enhanced Pseudo-Random Number Generators [v25]

Roger Riggs rriggs at openjdk.java.net
Fri Feb 26 21:53:46 UTC 2021


On Tue, 23 Feb 2021 16:47:59 GMT, Jim Laskey <jlaskey at openjdk.org> wrote:

>> This PR is to introduce a new random number API for the JDK. The primary API is found in RandomGenerator and RandomGeneratorFactory. Further description can be found in the JEP https://openjdk.java.net/jeps/356 .
>> 
>> javadoc can be found at http://cr.openjdk.java.net/~jlaskey/prng/doc/api/java.base/java/util/random/package-summary.html
>> 
>> old PR:  https://github.com/openjdk/jdk/pull/1273
>
> Jim Laskey has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 57 commits:
> 
>  - Merge branch 'master' into 8248862
>  - Adjust ThreadLocalRandom javadoc inheritence
>  - L32X64StarStarRandom -> L32X64MixRandom
>  - Various corrects
>  - Revised javadoc per CSR reviews
>  - Remove tabs from random/package-info.java
>  - Correct copyright notice.
>  - Merge branch 'master' into 8248862
>  - Update tests for RandomGeneratorFactory.all()
>  - Merge branch 'master' into 8248862
>  - ... and 47 more: https://git.openjdk.java.net/jdk/compare/0257caad...b9094279

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 232:

> 230:         Provider<? extends RandomGenerator> provider = fm.get(name);
> 231:         if (!isSubclass(category, provider)) {
> 232:             throw new IllegalArgumentException(name + " is an unknown random number generator");

The message is a bit vague. How about:

"The random number generator does not support the category"

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 240:

> 238:      * Returns a {@link RandomGenerator} that utilizes the {@code name}
> 239:      * <a href="package-summary.html#algorithms">algorithm</a>.
> 240:      *

In javadoc, this seems like is should read as: "utilizes the named algorithm".
The use of the parameter name is unnecessary in this case because it is obvious and readability suffers as is.

src/java.base/share/classes/java/util/random/RandomGenerator.java line 1313:

> 1311:      * furthermore can easily <i>jump</i> to an arbitrarily specified distant
> 1312:      * point in the state cycle.
> 1313:      *

The similarity of the first sentence of each of the Jumpable, Leapable, and arbitrarlyJumpable interface is so similar as to obscure the differences. You have to read 25 words in to be able to find the description that makes them different. The italics help but should include the whole of the phrase that distinguishes them.
Alternatively, move the phrase to the beginning of the sentence or drop the redundant phrasing.
"provide a common protocol of objects that generate pseudorandom sequences of numbers of Boolean values", etc.

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 55:

> 53:  *
> 54:  * A specific {@link RandomGeneratorFactory} can be located by using the
> 55:  * {@link RandomGenerator#factoryOf(String)} method, where the argument string

Broken link: the method is in this class.  should be just "#factoryOf".

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 600:

> 598:         try {
> 599:             ensureConstructors();
> 600:             return ctorBytes.newInstance((Object)seed);

IntelliJ warns that the cast to (Object) is redundant.

src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 29:

> 27: 
> 28: import java.lang.reflect.Constructor;
> 29: import java.lang.reflect.Method;

Used import.

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

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


More information about the security-dev mailing list