Collections.shuffle to accept RandomGenerator

[Paul Sandoz]

That looks reasonable.

Thinking a little more broadly.

We could also change Collections.shuffle(List) to use ThreadLocalRandom so it does not have to cache the Random instance, plus it has better concurrent and PRNG properties. The spec does not describe the precise details of randomness.
 
It’s tempting to lift Collections.shuffle(List, RandomGenerator) to List.shuffle(RandomGenerator ), similar to what we did for Collections.sort, to align with that pattern and which also aligns with reverse of sequenced collections. There is likely not much performance advantage to do doing so as there was with sort, but that location is much easier to find and use IMHO. Since the method accepts RandomGenerator the compatibility risk would likely be low.

Paul.

> On Sep 27, 2022, at 3:11 AM, Tagir Valeev <amaembo at gmail.com> wrote:
> 
> Hello!
> 
> Currently, Collections.shuffle(List, Random) accepts an outdated
> Random class instead of RandomGenerator interface. It looks like,
> RandomGenerator would suffice. The code change looks trivial (aside
> from documentation and testing), as shuffle() implementation does not
> need anything except nextInt:
> 
> diff --git a/src/java.base/share/classes/java/util/Collections.java
> b/src/java.base/share/classes/java/util/Collections.java
> --- a/src/java.base/share/classes/java/util/Collections.java (revision
> cab590517bf705418c7376edd5d7066b13b6dde8)
> +++ b/src/java.base/share/classes/java/util/Collections.java (date
> 1664273240190)
> @@ -37,6 +37,7 @@
> import java.util.function.IntFunction;
> import java.util.function.Predicate;
> import java.util.function.UnaryOperator;
> +import java.util.random.RandomGenerator;
> import java.util.stream.IntStream;
> import java.util.stream.Stream;
> import java.util.stream.StreamSupport;
> @@ -454,8 +455,12 @@
>      * @throws UnsupportedOperationException if the specified list or its
>      *         list-iterator does not support the {@code set} operation.
>      */
> -    @SuppressWarnings({"rawtypes", "unchecked"})
>     public static void shuffle(List<?> list, Random rnd) {
> +        shuffle(list, (RandomGenerator) rnd);
> +    }
> +
> +    @SuppressWarnings({"rawtypes", "unchecked"})
> +    public static void shuffle(List<?> list, RandomGenerator rnd) {
>         int size = list.size();
>         if (size < SHUFFLE_THRESHOLD || list instanceof RandomAccess) {
>             for (int i=size; i>1; i--)
> 
> What do you think? Should we implement this improvement? I think I can
> contribute if there's a general agreement that such an enhancement
> would be useful.
> 
> With best regards,
> Tagir Valeev