On Wed, 26 Jan 2022 15:36:28 GMT, Roger Riggs <rri...@openjdk.org> 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 > > src/java.base/share/classes/java/util/random/RandomGenerator.java line 144: > >> 142: /** >> 143: * Returns a wrapper to use {@link java.util.Random} >> 144: * @return {@link java.util.Random} > > Possible reword: > "Returns a Random from this RandomGenerator." > > It should be specified that the resulting Random acts in all respects except > that > setSeed is not available. > > Missing period at the end of the sentence. Check all comments for grammar > and punctuation. Is something like this better? /** * Returns a {@link java.util.Random} from this {@link java.util.RandomGenerator}. * The resulting Random acts in all respects except that setSeed is not available. * * @return {@link java.util.Random} */ > src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line > 41: > >> 39: @SuppressWarnings("serial") >> 40: public class RandomWrapper extends Random implements RandomGenerator { >> 41: private final RandomGenerator randomToWrap; > > Does this class need to be public? It acts as Random in almost all respects. Since it is in jdk.internal it needs to be public to use it in RandomGenerator interface, unless i move it to java.util > src/java.base/share/classes/jdk/internal/util/random/RandomWrapper.java line > 58: > >> 56: * setSeed does not exist in {@link >> java.util.random.RandomGenerator} so can't >> 57: * use it >> 58: */ > > Its debatable whether setSeed can ignore the argument or should throw an > exception. > If it ignores the argument, it is explicitly violating the contract of the > supertype Random.setSeed > that says the seed is set. > I'd recommend throwing an UnsupportedOperationException because setSeed will > not work as expected. > Other classes (ThreadLocalRandom) throws UnsupportedOperationException from > setSeed. throwing UnsupportedOperationException was my first thought but when i call asRandom() it will always throw it, i could create something like what ThreadLocalRandom does (check if it initialized and throw) ------------- PR: https://git.openjdk.java.net/jdk/pull/7001
