Simplify random and shuffle algos

laravel · Feb 14, 2023 · 9f39ac6 · 9f39ac6
1 parent 6e7534e
commit 9f39ac6
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 13 deletions.
diff --git a/src/Illuminate/Collections/Arr.php b/src/Illuminate/Collections/Arr.php
@@ -635,8 +635,7 @@ public static function random($array, $number = null, $preserveKeys = false)
         }
 
         if (is_null($number)) {
-            $values = array_values($array);
-            return $values[random_int(0, $count - 1)];
+            return head(array_slice($array, random_int(0, $count - 1), 1));
         }
 
         if ((int) $number === 0) {
@@ -704,18 +703,12 @@ public static function shuffle($array, $seed = null)
         }
 
         $keys = array_keys($array);
-
-        for ($i = count($keys) - 1; $i > 0; $i--) {
-            $j = random_int(0, $i);
-            $temp = $keys[$i];
-            $keys[$i] = $keys[$j];
-            $keys[$j] = $temp;
-        }
-
         $shuffled = [];
 
-        foreach ($keys as $key) {
-            $shuffled[$key] = $array[$key];
+        for ($i = count($keys) - 1; $i >= 0; $i--) {
+            $j = random_int(0, $i);
+            $shuffled[$keys[$j]] = $array[$keys[$j]];
+            $keys[$j] = $keys[$i];
         }
 
         return $shuffled;

diff --git a/tests/Support/SupportArrTest.php b/tests/Support/SupportArrTest.php
@@ -838,12 +838,39 @@ public function testShuffleWithSeed()
 
     public function testShuffle()
     {
+        $this->assertEquals(
+            Arr::shuffle(range(0, 10)),
+            Arr::shuffle(range(0, 10)),
+            'Shuffled array should have the same elements.'
+        );
+
         $this->assertNotSame(
             Arr::shuffle(range(0, 10)),
-            Arr::shuffle(range(0, 10))
+            Arr::shuffle(range(0, 10)),
+            'Shuffled array should not have the same order.'
         );
     }
 
+    public function testShuffleWithKeys()
+    {
+        $array = ['one' => 'foo', 'two' => 'bar', 'three' => 'baz', 'four' => 'boom'];
+
+        $sameElements = true;
+        $dontMatch = false;
+
+        // Attempt 5x times to prevent random failures
+        for ($i = 0; $i < 5; $i++) {
+            $one = Arr::shuffle($array);
+            $two = Arr::shuffle($array);
+
+            $sameElements = $sameElements && $one == $two;
+            $dontMatch = $dontMatch || $one !== $two;
+        }
+
+        $this->assertTrue($sameElements, 'Shuffled array should always have the same elements.');
+        $this->assertTrue($dontMatch, 'Shuffled array should not have the same order.');
+    }
+
     public function testSort()
     {
         $unsorted = [