fix(combinations-replacement): return an empty list for a non-positive group size
combinationsReplacement recurses on x.slice(i, x.length), which at i = 0 is as long as the input, so the only exit left is the k === 1 base case. For k <= 0 the counter starts below that case and moves away from it, and the recursion overflows the stack. The sibling combinations recurses on x.slice(i + 1, x.length), runs out of input and returns [] for the same arguments. Guard the non-positive group size up front so both functions agree.
This commit is contained in:
1 parent
1aa682fd34
commit
24fe35e78e
3 files changed
+38
No files matched your search
@@ -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.
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user