From 8abbc792f613611a7cdb642c42349d83dc89f4ba Mon Sep 17 00:00:00 2001 From: siiky Date: Fri, 31 Jan 2025 13:19:05 +0000 Subject: [PATCH 1/4] fix: target without divisor denoms is no shortcut condition --- lib/coin-change.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/coin-change.js b/lib/coin-change.js index 1c9eab5e..4a1c964a 100644 --- a/lib/coin-change.js +++ b/lib/coin-change.js @@ -24,11 +24,6 @@ const prepare_denominations = denominations => const max_denomination_multiplicity = (denom, count, target) => Math.min(count, Math.floor(target / denom)) -const has_divisor = (didx, denominations, target) => - denominations - .slice(didx) - .some(({ denom }) => (target % denom) === 0) - /* * @returns null if there's no solution set; * false if there's no solution; @@ -78,7 +73,7 @@ const solve = (model, target) => { * of the denominations, or if the target is not divisible by any of the * denominations */ - if (target > csum || !has_divisor(didx, denominations, target)) + if (target > csum) return memo_set(memo, target, denom, false) let solution = memo_get(memo, target, denom) From 91c4ca4dceb65a7b92dab21762a1745d677afa0b Mon Sep 17 00:00:00 2001 From: siiky Date: Mon, 3 Feb 2025 14:34:19 +0000 Subject: [PATCH 2/4] fix: ensure `coin-change` is valid before comparing with `subset-sum` --- lib/bill-math.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/bill-math.js b/lib/bill-math.js index 8f5daea5..b093fd46 100644 --- a/lib/bill-math.js +++ b/lib/bill-math.js @@ -160,13 +160,13 @@ function makeChange(outCassettes, amount) { const ss_solution = getSolution_old(outCassettes, amount, BILL_LIST_MODES.VALUE_ROUND_ROBIN) const cc_solution = getSolution(outCassettes, amount) - if (!!ss_solution !== !!cc_solution) { - logger.error(new Error(`subset-sum and coin-change don't agree on solvability -- subset-sum:${!!ss_solution} coin-change:${!!cc_solution}`)) + if (!cc.check(cc_solution, amount.toNumber())) { + logger.error(new Error("coin-change provided a bad solution")) return solutionToOriginalUnits(ss_solution, outCassettes) } - if (!cc.check(cc_solution, amount.toNumber())) { - logger.error(new Error("coin-change provided a bad solution")) + if (!!ss_solution !== !!cc_solution) { + logger.error(new Error(`subset-sum and coin-change don't agree on solvability -- subset-sum:${!!ss_solution} coin-change:${!!cc_solution}`)) return solutionToOriginalUnits(ss_solution, outCassettes) } From 9ef9a70c39cf097219eb8c8e5ea9b661562b356e Mon Sep 17 00:00:00 2001 From: siiky Date: Mon, 3 Feb 2025 14:36:30 +0000 Subject: [PATCH 3/4] fix: use the same solution format for `subset-sum` as for `coin-change` --- lib/bill-math.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/bill-math.js b/lib/bill-math.js index b093fd46..8d4515c2 100644 --- a/lib/bill-math.js +++ b/lib/bill-math.js @@ -116,14 +116,8 @@ const getSolution_old = (units, amount, mode) => { const solver = sumService.subsetSum(billList, amount.toNumber()) const solution = _.countBy(Math.floor, solver.next().value) - return _.reduce( - (acc, value) => { - acc.push({ denomination: _.toNumber(value), provisioned: solution[value] }) - return acc - }, - [], - _.keys(solution) - ) + return Object.entries(solution) + .map(([denomination, provisioned]) => [_.toNumber(denomination), provisioned]) } const getSolution = (units, amount) => { From 16e57ee3aba3185e0fe257cf14b54cd0e7137c53 Mon Sep 17 00:00:00 2001 From: siiky Date: Mon, 3 Feb 2025 14:35:16 +0000 Subject: [PATCH 4/4] refactor: simplify reducing solution --- lib/bill-math.js | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/bill-math.js b/lib/bill-math.js index 8d4515c2..f83ff49c 100644 --- a/lib/bill-math.js +++ b/lib/bill-math.js @@ -129,24 +129,13 @@ const getSolution = (units, amount) => { const solutionToOriginalUnits = (solution, units) => { const billsToAssign = (count, left) => _.clamp(0, count)(_.isNaN(left) || _.isNil(left) ? 0 : left) - - const billsLeft = _.flow( - _.map(([denomination, provisioned]) => [BN(denomination), provisioned]), - _.reduce((acc, value) => { - acc[value[0]] = (acc[value[0]] || BN(0)).plus(value[1]) - return acc - }, - {} - ) - )(solution) - - return _.map( + const billsLeft = Object.fromEntries(solution) + return units.map( ({ count, name, denomination }) => { const provisioned = billsToAssign(count, billsLeft[denomination]) billsLeft[denomination] -= provisioned return { name, denomination, provisioned } - }, - units + } ) }