diff --git a/.changeset/combinations-replacement-non-positive-k.md b/.changeset/combinations-replacement-non-positive-k.md new file mode 100644 index 0000000..ee48fb1 --- /dev/null +++ b/.changeset/combinations-replacement-non-positive-k.md @@ -0,0 +1,5 @@ +--- +"simple-statistics": patch +--- + +Fix `combinationsReplacement` overflowing the stack for a non-positive group size. `combinations` recurses on `x.slice(i + 1, x.length)`, so it always runs out of input and returns `[]` for `k <= 0`. `combinationsReplacement` recurses on `x.slice(i, x.length)`, which at `i = 0` is as long as `x`, so with `k` counting down past its `k === 1` base case the recursion never terminates: `combinationsReplacement([1, 2, 3], 0)` and `combinationsReplacement([1, 2, 3], -1)` both threw `RangeError: Maximum call stack size exceeded`. Both now return `[]`, matching the sibling function. diff --git a/src/combinations_replacement.js b/src/combinations_replacement.js index 1fc5832..2a54fe5 100644 --- a/src/combinations_replacement.js +++ b/src/combinations_replacement.js @@ -13,6 +13,16 @@ function combinationsReplacement(x, k) { const combinationList = []; + // There is no group of zero or fewer objects to draw, so there is nothing + // to enumerate. This case has to be caught up front: unlike `combinations`, + // which recurses on `x.slice(i + 1, x.length)` and therefore always runs + // out of input, the recursion below re-uses `x.slice(i, x.length)`, which + // at `i = 0` is just as long as `x`. With `k` never reaching 1, that + // recursion never terminates and overflows the stack. + if (k <= 0) { + return combinationList; + } + for (let i = 0; i < x.length; i++) { if (k === 1) { // If we're requested to find only one element, we don't need diff --git a/test/combinations_replacement.test.js b/test/combinations_replacement.test.js index 4265341..5b034e1 100644 --- a/test/combinations_replacement.test.js +++ b/test/combinations_replacement.test.js @@ -15,4 +15,27 @@ describe("combinations", function () { [2, 2] ]); }); + it("returns no combinations when choosing zero at a time", function () { + assert.deepEqual( + ss.combinationsReplacement(Object.freeze([1, 2, 3]), 0), + [] + ); + }); + it("returns no combinations for a negative group size", function () { + assert.deepEqual( + ss.combinationsReplacement(Object.freeze([1, 2, 3]), -1), + [] + ); + }); + it("agrees with combinations on non-positive group sizes", function () { + const x = Object.freeze([1, 2, 3]); + assert.deepEqual( + ss.combinationsReplacement(x, 0), + ss.combinations(x, 0) + ); + assert.deepEqual( + ss.combinationsReplacement(x, -1), + ss.combinations(x, -1) + ); + }); });