laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.62k stars 11.03k forks source link

Arr::random() always returns values in same order as in source array #53599

Closed vazaha-nl closed 1 day ago

vazaha-nl commented 1 day ago

Laravel Version

11.33.2

PHP Version

8.2.24

Database Driver & Version

No response

Description

I noticed that the method \Illuminate\Support\Arr::random() always returns elements in exactly the same order as in the source array if called with $number greater than 1. This seems highly counterintuitive, even wrong to me. I would expect an array with the specified number of random items, in random order. Is my expectiation way off or is this indeed a bug?

Steps To Reproduce

// both expressions will always return true
Arr::random(['A', 'B', 'C'], 3) === ['A', 'B', 'C'];
Arr::random(['C', 'B', 'A'], 3) === ['C', 'B', 'A'];
rodrigopedra commented 1 day ago

Arr::random() uses \Random\Randomizer@pickArrayKeys under the hood.

In the \Random\Randomizer docs, it says \Random\Randomizer@pickArrayKeys will return:

An array containing num distinct array keys of array.

Reference: https://www.php.net/manual/en/random-randomizer.pickarraykeys.php

It is not clear within the docs, but from my testing, the returned array seems always sorted.

Maybe that hast to be on the enforcing unique/distinct array keys.

So, when the number of random array keys requested is equal to the number of elements of an array, one ends up getting the same elements in the same order.

If you test with a 4-element array, and request 3 elements, you will see that you will get 33 different elements back. Even though, they will appear in the same order as in the original array.

It seems "random" means random picks, not random order. So when the picks' size equals the sample's size, you end up with the same sample.

I agree that is unexpected, and odd. But I wouldn't blame Laravel on this. The \Random\Randomizer class is a built-in PHP class since PHP 8.2, and that behavior might be like that by design.

As a workaround, you can call Arr::shuffle after Arr::random, to shuffle the result set order.

Or propose a PR to shuffle the keys after Arr::random() calls \Random\Randomizer@pickArrayKeys internally.

valorin commented 1 day ago

Agreed, it's not technically a bug due to how the function works: pick a random selection of X elements.

But, I can see a benefit in shuffling the picked elements too, and that this behaviour might not be expected or even desired here.

You could PR a fix and see what sort of feedback that gets?

vazaha-nl commented 1 day ago

I initially encountered this issue in an older version of laravel, v10.48.23. There the function uses array_rand and there the documentation explicitly states: "If multiple keys are returned, they will be returned in the order they were present in the original array" which is the reason the values have the same order as well.