refactor(helpers)!: rewrite shuffle (#1521)

Co-authored-by: ST-DDT <ST-DDT@gmx.de>
faker-js · Nov 7, 2022 · a5de229 · a5de229
1 parent 7e00d17
commit a5de229
Show file tree

Hide file tree

Showing 3 changed files with 196 additions and 27 deletions.
diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts
@@ -240,29 +240,61 @@ export class HelpersModule {
   /**
    * Takes an array and randomizes it in place then returns it.
    *
-   * Uses the modern version of the Fisher–Yates algorithm.
+   * @template T The type of the entries to shuffle.
+   * @param list The array to shuffle.
+   * @param options The options to use when shuffling.
+   * @param options.inplace Whether to shuffle the array in place or return a new array. Defaults to `false`.
+   *
+   * @example
+   * faker.helpers.shuffle(['a', 'b', 'c'], { inplace: true }) // [ 'b', 'c', 'a' ]
+   *
+   * @since 8.0.0
+   */
+  shuffle<T>(list: T[], options: { inplace: true }): T[];
+  /**
+   * Returns a randomized version of the array.
    *
    * @template T The type of the entries to shuffle.
-   * @param o The array to shuffle. Defaults to `[]`.
+   * @param list The array to shuffle.
+   * @param options The options to use when shuffling.
+   * @param options.inplace Whether to shuffle the array in place or return a new array. Defaults to `false`.
    *
    * @example
-   * faker.helpers.shuffle() // []
    * faker.helpers.shuffle(['a', 'b', 'c']) // [ 'b', 'c', 'a' ]
+   * faker.helpers.shuffle(['a', 'b', 'c'], { inplace: false }) // [ 'b', 'c', 'a' ]
    *
    * @since 2.0.1
    */
-  shuffle<T>(o?: T[]): T[] {
-    if (o == null || o.length === 0) {
-      return o || [];
+  shuffle<T>(list: readonly T[], options?: { inplace?: false }): T[];
+  /**
+   * Returns a randomized version of the array.
+   *
+   * @template T The type of the entries to shuffle.
+   * @param list The array to shuffle.
+   * @param options The options to use when shuffling.
+   * @param options.inplace Whether to shuffle the array in place or return a new array. Defaults to `false`.
+   *
+   * @example
+   * faker.helpers.shuffle(['a', 'b', 'c']) // [ 'b', 'c', 'a' ]
+   * faker.helpers.shuffle(['a', 'b', 'c'], { inplace: true }) // [ 'b', 'c', 'a' ]
+   * faker.helpers.shuffle(['a', 'b', 'c'], { inplace: false }) // [ 'b', 'c', 'a' ]
+   *
+   * @since 2.0.1
+   */
+  shuffle<T>(list: T[], options?: { inplace?: boolean }): T[];
+  shuffle<T>(list: T[], options: { inplace?: boolean } = {}): T[] {
+    const { inplace = false } = options;
+
+    if (!inplace) {
+      list = [...list];
     }
 
-    for (let i = o.length - 1; i > 0; --i) {
+    for (let i = list.length - 1; i > 0; --i) {
       const j = this.faker.datatype.number(i);
-      const x = o[i];
-      o[i] = o[j];
-      o[j] = x;
+      [list[i], list[j]] = [list[j], list[i]];
     }
-    return o;
+
+    return list;
   }
 
   /**

diff --git a/test/__snapshots__/helpers.spec.ts.snap b/test/__snapshots__/helpers.spec.ts.snap
@@ -69,8 +69,6 @@ exports[`helpers > 42 > replaceSymbols > only symbols 1`] = `"3U17U5"`;
 
 exports[`helpers > 42 > replaceSymbols > some string 1`] = `"^1234567890ß´°!\\"§$%&/()=J\`+71,..-;:_"`;
 
-exports[`helpers > 42 > shuffle > noArgs 1`] = `[]`;
-
 exports[`helpers > 42 > shuffle > with array 1`] = `
 [
   "!",
@@ -88,6 +86,40 @@ exports[`helpers > 42 > shuffle > with array 1`] = `
 ]
 `;
 
+exports[`helpers > 42 > shuffle > with array and inplace false 1`] = `
+[
+  "!",
+  "W",
+  "d",
+  "H",
+  "l",
+  "l",
+  "o",
+  " ",
+  "e",
+  "l",
+  "r",
+  "o",
+]
+`;
+
+exports[`helpers > 42 > shuffle > with array and inplace true 1`] = `
+[
+  "!",
+  "W",
+  "d",
+  "H",
+  "l",
+  "l",
+  "o",
+  " ",
+  "e",
+  "l",
+  "r",
+  "o",
+]
+`;
+
 exports[`helpers > 42 > slugify > noArgs 1`] = `""`;
 
 exports[`helpers > 42 > slugify > some string 1`] = `"hello-world"`;
@@ -185,8 +217,6 @@ exports[`helpers > 1211 > replaceSymbols > only symbols 1`] = `"9L72D0"`;
 
 exports[`helpers > 1211 > replaceSymbols > some string 1`] = `"^1234567890ß´°!\\"§$%&/()=Y\`+47,..-;:_"`;
 
-exports[`helpers > 1211 > shuffle > noArgs 1`] = `[]`;
-
 exports[`helpers > 1211 > shuffle > with array 1`] = `
 [
   "l",
@@ -204,6 +234,40 @@ exports[`helpers > 1211 > shuffle > with array 1`] = `
 ]
 `;
 
+exports[`helpers > 1211 > shuffle > with array and inplace false 1`] = `
+[
+  "l",
+  "l",
+  "o",
+  "l",
+  "W",
+  "d",
+  "H",
+  "e",
+  "o",
+  "r",
+  " ",
+  "!",
+]
+`;
+
+exports[`helpers > 1211 > shuffle > with array and inplace true 1`] = `
+[
+  "l",
+  "l",
+  "o",
+  "l",
+  "W",
+  "d",
+  "H",
+  "e",
+  "o",
+  "r",
+  " ",
+  "!",
+]
+`;
+
 exports[`helpers > 1211 > slugify > noArgs 1`] = `""`;
 
 exports[`helpers > 1211 > slugify > some string 1`] = `"hello-world"`;
@@ -291,8 +355,6 @@ exports[`helpers > 1337 > replaceSymbols > only symbols 1`] = `"2OF2OA"`;
 
 exports[`helpers > 1337 > replaceSymbols > some string 1`] = `"^1234567890ß´°!\\"§$%&/()=G\`+5F,..-;:_"`;
 
-exports[`helpers > 1337 > shuffle > noArgs 1`] = `[]`;
-
 exports[`helpers > 1337 > shuffle > with array 1`] = `
 [
   " ",
@@ -310,6 +372,40 @@ exports[`helpers > 1337 > shuffle > with array 1`] = `
 ]
 `;
 
+exports[`helpers > 1337 > shuffle > with array and inplace false 1`] = `
+[
+  " ",
+  "d",
+  "o",
+  "r",
+  "H",
+  "o",
+  "!",
+  "l",
+  "l",
+  "e",
+  "W",
+  "l",
+]
+`;
+
+exports[`helpers > 1337 > shuffle > with array and inplace true 1`] = `
+[
+  " ",
+  "d",
+  "o",
+  "r",
+  "H",
+  "o",
+  "!",
+  "l",
+  "l",
+  "e",
+  "W",
+  "l",
+]
+`;
+
 exports[`helpers > 1337 > slugify > noArgs 1`] = `""`;
 
 exports[`helpers > 1337 > slugify > some string 1`] = `"hello-world"`;

diff --git a/test/helpers.spec.ts b/test/helpers.spec.ts
@@ -66,7 +66,13 @@ describe('helpers', () => {
     });
 
     t.describe('shuffle', (t) => {
-      t.it('noArgs').it('with array', 'Hello World!'.split(''));
+      t.it('with array', 'Hello World!'.split(''))
+        .it('with array and inplace true', 'Hello World!'.split(''), {
+          inplace: true,
+        })
+        .it('with array and inplace false', 'Hello World!'.split(''), {
+          inplace: false,
+        });
     });
 
     t.describe('uniqueArray', (t) => {
@@ -306,26 +312,61 @@ describe('helpers', () => {
 
         it('mutates the input array in place', () => {
           const input = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
-          const shuffled = faker.helpers.shuffle(input);
+          const shuffled = faker.helpers.shuffle(input, { inplace: true });
           expect(shuffled).deep.eq(input);
         });
 
-        it('all items shuffled as expected when seeded', () => {
-          const input = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j'];
-          faker.seed(100);
-          const shuffled = faker.helpers.shuffle(input);
-          expect(shuffled).deep.eq([
+        it('does not mutate the input array by default', () => {
+          const input = Object.freeze([
+            'a',
             'b',
+            'c',
+            'd',
             'e',
+            'f',
+            'g',
+            'h',
+            'i',
+            'j',
+          ]);
+          expect(() => faker.helpers.shuffle(input)).not.to.throw();
+        });
+
+        it('does not mutate the input array when inplace is false', () => {
+          const input = Object.freeze([
             'a',
+            'b',
+            'c',
             'd',
-            'j',
-            'i',
+            'e',
+            'f',
+            'g',
             'h',
+            'i',
+            'j',
+          ]);
+          expect(() =>
+            faker.helpers.shuffle(input, { inplace: false })
+          ).not.to.throw();
+        });
+
+        it('throws an error when the input array is readonly and inplace is true', () => {
+          const input = Object.freeze([
+            'a',
+            'b',
             'c',
-            'g',
+            'd',
+            'e',
             'f',
+            'g',
+            'h',
+            'i',
+            'j',
           ]);
+          expect(() =>
+            // @ts-expect-error: we want to test that it throws
+            faker.helpers.shuffle(input, { inplace: true })
+          ).to.throw();
         });
       });