On Mon, 13 May 2024 13:14:57 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> Thinking a bit more, I think we can even get rid of the reflection used in
>> `create()` methods implementation, if we wanted to, by doing something like
>> this:
>>
>>
>> private record RandomGenEntry(Class<? extends RandomGenerator>
>> randomGenClass, int i, int j,
>> int k, int equiDistribution, boolean
>> stochastic,
>> boolean hardware) {
>>
>> RandomGenerator create() {
>> String algo = randomGenClass.getSimpleName();
>> return switch (algo) {
>> case "Random" -> new Random();
>> case "L128X1024MixRandom" -> new L128X1024MixRandom();
>> case "Xoshiro256PlusPlus" -> new Xoshiro256PlusPlus();
>> // ... so on for the rest
>> default -> throw new InternalError("should not happen");
>> };
>> }
>>
>> RandomGenerator create(long seed) {
>> String algo = randomGenClass.getSimpleName();
>> return switch (algo) {
>> case "Random", "SplittableRandom", "SecureRandom" -> {
>> throw new UnsupportedOperationException("cannot
>> construct with a long seed");
>> }
>> case "L128X1024MixRandom" -> new L128X1024MixRandom(seed);
>> case "Xoshiro256PlusPlus" -> new Xoshiro256PlusPlus(seed);
>> // ... so on for the rest
>> default -> throw new InternalError("should not happen");
>> };
>> }
>>
>> RandomGenerator create(byte[] seed) {
>> String algo = randomGenClass.getSimpleName();
>> return switch (algo) {
>> case "Random", "SplittableRandom", "SecureRandom" -> {
>> throw new UnsupportedOperationException("cannot
>> construct with a byte[] seed");
>> }
>> case "L128X1024MixRandom" -> new L128X1024MixRandom(seed);
>> case "Xoshiro256PlusPlus" -> new Xoshiro256PlusPlus(seed);
>> // ... so on for the rest
>> default -> throw new InternalError("should not happen");
>> };
>> }
>> }
>>
>>
>> private static final Map<String, RandomGenEntry> FACTORY_MAP = ... //
>> construct the map
>
> @jaikiran I agree that we can simplify even more. I just wanted to change as
> little as possible in this PR to facilitate reviews.
> Shall I push your proposed changes in this PR or is a followup PR preferable?
A followup PR is fine. I think once this PR which addresses the API aspects
(like removal of ServiceLoader and then updates to the create() method javadoc)
is integrated, then the subsequent PR can just be all internal implementation
changes like the proposed removal of reflection.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19212#discussion_r1598464086
