On Fri, 26 Feb 2021 13:15:28 GMT, Rémi Forax <github.com+828220+fo...@openjdk.org> wrote:
>> Looking for some final code reviews. > > I still don't like the fact that the factory uses reflection but i don't see > how to do better This is now looking very nicely structured. The only thing i am unsure are the details around `RandomGenerator` being a service provider interface. The documentation mentions this at various points (mostly as implementation notes), but it's not really called out on `RandomGenerator`. Trying out the patch, I can implement `RandomGenerator` and register it as a service: public class AlwaysZero implements RandomGenerator { @Override public long nextLong() { return 0; } } ... RandomGenerator alwaysZero = RandomGenerator.of("AlwaysZero"); Is that intended? (esp. since the annotation `RandomGeneratorProperties` is not public). If i rename the above to `L32X64MixRandom` an `ExceptionInInitializerError` is produced due to duplicate keys. I suspect you want to filter out the service providers to those that only declare `RandomGeneratorProperties`, thereby restricting to providers only supplied by the platform. ------------- PR: https://git.openjdk.java.net/jdk/pull/1292