On Fri, 11 Mar 2022 01:25:28 GMT, Yasser Bazzi <d...@openjdk.java.net> wrote:
>> Hi, could i get a review on this implementation proposed by Stuart Marks, i >> decided to use the >> https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/util/random/RandomGenerator.html >> interface to create the default method `asRandom()` that wraps around the >> newer algorithms to be used on classes that do not accept the new interface. >> >> Some things to note as proposed by the bug report, the protected method >> next(int bits) is not overrided and setSeed() method if left blank up to >> discussion on what to do with it. >> >> Small test done on >> https://gist.github.com/YShow/da678561419cda8e32fccf3a27a649d4 > > Yasser Bazzi has updated the pull request incrementally with two additional > commits since the last revision: > > - Merge branch 'randomwrapper' of https://github.com/YShow/jdk.git into > randomwrapper > - Remove atomicLong allocation and override next(int) method to throw UOE A small spec change is necessary; please make it here, and I'll copy it into the CSR. src/java.base/share/classes/java/util/Random.java line 98: > 96: private RandomWrapper(RandomGenerator randomToWrap) { > 97: super(null); > 98: this.generator = Objects.requireNonNull(randomToWrap); Can remove `requireNonNull` here; see other comments. At your option, add a comment saying that `randomToWrap` must not be null. This is easy to verify since there's only one caller elsewhere in this file. src/java.base/share/classes/java/util/Random.java line 320: > 318: * @param generator the {@code RandomGenerator} calls are delegated > to > 319: * @return the delegating {@code Random} instance > 320: */ Need to add @throws NullPointerException if generator is null src/java.base/share/classes/java/util/Random.java line 327: > 325: return new RandomWrapper(generator); > 326: } > 327: I'd suggest adding Objects.requireNonNull(generator) as the first line of this method. In fact if null is passed, NPE is thrown already, as the `instanceof` will be false and then the `RandomWrapper` constructor will throw NPE. However, verifying this requires a bit of hunting around and some flow analysis to determine this. Might as well make the null check explicit at the top of this method. The now-redundant check can be removed from `RandomWrapper`. ------------- PR: https://git.openjdk.java.net/jdk/pull/7001
