From b2c4467f55c130b8d90c91940f2b63a38da7a9de Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 4 Nov 2022 18:27:15 +0100 Subject: [PATCH 1/4] refactor(helpers)!: rewrite shuffle --- src/modules/helpers/index.ts | 53 ++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index b8bbb725bda..5c78d6dce67 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -240,29 +240,60 @@ 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 2.0.1 + */ + shuffle(list: T[], options: { inplace: true }): 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: false }) // [ 'b', 'c', 'a' ] + * + * @since 2.0.1 + */ + shuffle(list: readonly T[], options?: { inplace?: false }): 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: true }) // [ 'b', 'c', 'a' ] + * faker.helpers.shuffle(['a', 'b', 'c'], { inplace: false }) // [ 'b', 'c', 'a' ] * * @since 2.0.1 */ - shuffle(o?: T[]): T[] { - if (o == null || o.length === 0) { - return o || []; + shuffle(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; } /** From 6c25f1b0e432c721f4f3033ab97ef4397732feb3 Mon Sep 17 00:00:00 2001 From: Shinigami92 Date: Fri, 4 Nov 2022 18:55:23 +0100 Subject: [PATCH 2/4] test: update tests --- src/modules/helpers/index.ts | 1 + test/__snapshots__/helpers.spec.ts.snap | 108 ++++++++++++++++++++++-- test/helpers.spec.ts | 61 ++++++++++--- 3 files changed, 154 insertions(+), 16 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index 5c78d6dce67..b82969d38c4 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -281,6 +281,7 @@ export class HelpersModule { * * @since 2.0.1 */ + shuffle(list: T[], options?: { inplace?: boolean }): T[]; shuffle(list: T[], options?: { inplace?: boolean }): T[] { const { inplace = false } = options || {}; diff --git a/test/__snapshots__/helpers.spec.ts.snap b/test/__snapshots__/helpers.spec.ts.snap index fcac31217c8..6c6c31bdcd9 100644 --- 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 index 39aa1982ee5..a8a89b3ad2d 100644 --- 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(); }); }); From 41a4ab94418f715d5d5a233e6a172bb587da862b Mon Sep 17 00:00:00 2001 From: Shinigami Date: Fri, 4 Nov 2022 20:03:54 +0100 Subject: [PATCH 3/4] chore: apply review suggestion Co-authored-by: ST-DDT --- src/modules/helpers/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index b82969d38c4..b2bd61b5d76 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -282,8 +282,8 @@ export class HelpersModule { * @since 2.0.1 */ shuffle(list: T[], options?: { inplace?: boolean }): T[]; - shuffle(list: T[], options?: { inplace?: boolean }): T[] { - const { inplace = false } = options || {}; + shuffle(list: T[], options: { inplace?: boolean } = {}): T[] { + const { inplace = false } = options; if (!inplace) { list = [...list]; From e728e6dc622e9f6a115539e39c4ceb85d9785203 Mon Sep 17 00:00:00 2001 From: Shinigami Date: Mon, 7 Nov 2022 16:46:45 +0100 Subject: [PATCH 4/4] chore: change since tag --- src/modules/helpers/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/helpers/index.ts b/src/modules/helpers/index.ts index b2bd61b5d76..063ec8d3c21 100644 --- a/src/modules/helpers/index.ts +++ b/src/modules/helpers/index.ts @@ -248,7 +248,7 @@ export class HelpersModule { * @example * faker.helpers.shuffle(['a', 'b', 'c'], { inplace: true }) // [ 'b', 'c', 'a' ] * - * @since 2.0.1 + * @since 8.0.0 */ shuffle(list: T[], options: { inplace: true }): T[]; /**