From 5894b8b560ad2130cf7004d31e03d1d621413de4 Mon Sep 17 00:00:00 2001 From: dualfroz Date: Thu, 3 Sep 2026 14:06:28 +0200 Subject: [PATCH] fix(permutations-heap): do the swaps on a copy of the input Heap's Algorithm generates each permutation by swapping two elements of a working array in place, and that working array was the argument itself, so permutationsHeap([1, 2, 3]) left the caller holding [3, 2, 1]. Two calls on one array returned different lists, and a frozen array threw. Copy the input up front and swap on the copy, as shuffle and numericSort already do. The returned permutations are unchanged. --- .../permutations-heap-aliases-caller-array.md | 5 ++++ src/permutations_heap.js | 23 ++++++++++++------- test/permutations_heap.test.js | 15 ++++++++++++ 3 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 .changeset/permutations-heap-aliases-caller-array.md diff --git a/.changeset/permutations-heap-aliases-caller-array.md b/.changeset/permutations-heap-aliases-caller-array.md new file mode 100644 index 0000000..bb946f1 --- /dev/null +++ b/.changeset/permutations-heap-aliases-caller-array.md @@ -0,0 +1,5 @@ +--- +"simple-statistics": patch +--- + +Fix `permutationsHeap` permuting the caller's array. Heap's Algorithm works by swapping elements in place, and the implementation ran those swaps directly on the argument, so `permutationsHeap([1, 2, 3])` left the caller holding `[3, 2, 1]`, the last permutation generated. Calling it twice on the same array returned two different lists, and a frozen input threw `TypeError: Cannot assign to read only property '0'`. The swaps now run on a copy, matching the defensive copying already used in `shuffle`. The returned permutations are unchanged. diff --git a/src/permutations_heap.js b/src/permutations_heap.js index 1af6317..8256b0c 100644 --- a/src/permutations_heap.js +++ b/src/permutations_heap.js @@ -2,18 +2,25 @@ * Implementation of [Heap's Algorithm](https://en.wikipedia.org/wiki/Heap%27s_algorithm) * for generating permutations. * + * Heap's Algorithm permutes an array in place. This function does the swapping + * on a copy, so it will not modify its input. + * * @param {Array} elements any type of data * @returns {Array} array of permutations */ function permutationsHeap(elements) { - const indexes = new Array(elements.length); - const permutations = [elements.slice()]; + // slice the original array so that it is not modified: the swaps below + // are done in place + const sample = elements.slice(); - for (let i = 0; i < elements.length; i++) { + const indexes = new Array(sample.length); + const permutations = [sample.slice()]; + + for (let i = 0; i < sample.length; i++) { indexes[i] = 0; } - for (let i = 0; i < elements.length; ) { + for (let i = 0; i < sample.length; ) { if (indexes[i] < i) { // At odd indexes, swap from indexes[i] instead // of from the beginning of the array @@ -24,11 +31,11 @@ function permutationsHeap(elements) { // swap between swapFrom and i, using // a temporary variable as storage. - const temp = elements[swapFrom]; - elements[swapFrom] = elements[i]; - elements[i] = temp; + const temp = sample[swapFrom]; + sample[swapFrom] = sample[i]; + sample[i] = temp; - permutations.push(elements.slice()); + permutations.push(sample.slice()); indexes[i]++; i = 0; } else { diff --git a/test/permutations_heap.test.js b/test/permutations_heap.test.js index fbc06f0..02968a0 100644 --- a/test/permutations_heap.test.js +++ b/test/permutations_heap.test.js @@ -16,4 +16,19 @@ describe("permutationsHeap", function () { [3, 2, 1] ]); }); + it("does not modify the caller's array", function () { + const x = [1, 2, 3]; + ss.permutationsHeap(x); + assert.deepEqual(x, [1, 2, 3]); + }); + it("accepts a frozen array", function () { + assert.deepEqual(ss.permutationsHeap(Object.freeze([1, 2])), [ + [1, 2], + [2, 1] + ]); + }); + it("returns the same permutations when called twice on one array", function () { + const x = [1, 2, 3]; + assert.deepEqual(ss.permutationsHeap(x), ss.permutationsHeap(x)); + }); });