Merge pull request #1783 from siiky/fix/lam-1296/coin-change

LAM-1296 fix: `coin-change`
lamassu · Feb 4, 2025 · f8e0ced · f8e0ced
2 parents 39d5ab3 + dd18fdc
commit f8e0ced
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 32 deletions.
diff --git 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) => {
@@ -135,38 +129,27 @@ 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
+    }
   )
 }
 
 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)
   }
 

diff --git 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)