On Wed, 25 Nov 2020 13:09:03 GMT, Jim Laskey <jlas...@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 incrementally with one additional > commit since the last revision: > > 8248862: Implement Enhanced Pseudo-Random Number Generators > > Changes to RandomGeneratorFactory requested by @PaulSandoz src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 151: > 149: if (fm == null) { > 150: synchronized (RandomGeneratorFactory.class) { > 151: if (RandomGeneratorFactory.factoryMap == null) { if `RandomGeneratorFactory.factoryMap` is not null, it returns null because the value is not stored in fm. Please use the class holder idiom (cf Effective Java) instead of using the double-check locking pattern. src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 157: > 155: .stream() > 156: .filter(p -> !p.type().isInterface()) > 157: .collect(Collectors.toMap(p -> > p.type().getSimpleName().toUpperCase(), toUpperCase() depends on the Locale ! src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 175: > 173: if (props == null) { > 174: synchronized (provider) { > 175: if (properties == null) { same issue with the double check locking src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 177: > 175: if (properties == null) { > 176: try { > 177: Method getProperties = > provider.type().getDeclaredMethod("getProperties"); Is it not a better way than using reflection here ? src/java.base/share/classes/java/util/random/RandomGeneratorFactory.java line 180: > 178: > PrivilegedExceptionAction<Map<RandomGeneratorProperty, Object>> getAction = > () -> { > 179: getProperties.setAccessible(true); > 180: return (Map<RandomGeneratorProperty, > Object>)getProperties.invoke(null); Given that we have no control over the fact that the method properties() doesn't return a Map of something else, this unsafe cast seems dangerous (from the type system POV). Having a type representing a reified version of the Map may be a better idea ------------- PR: https://git.openjdk.java.net/jdk/pull/1292
