Fix Money::allocateWithRemainder()

brick · Jun 19, 2022 · 75bc1e3 · 75bc1e3
1 parent e9ec761
commit 75bc1e3
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 10 deletions.
diff --git a/src/Money.php b/src/Money.php
@@ -16,6 +16,7 @@
 use Brick\Math\Exception\MathException;
 use Brick\Math\Exception\NumberFormatException;
 use Brick\Math\Exception\RoundingNecessaryException;
+use InvalidArgumentException;
 
 /**
  * A monetary value in a given currency. This class is immutable.
@@ -590,21 +591,75 @@ public function allocateWithRemainder(int ...$ratios) : array
             throw new \InvalidArgumentException('Cannot allocateWithRemainder() to zero ratios only.');
         }
 
-        $monies = [];
+        $ratios = $this->simplifyRatios(array_values($ratios));
+        $total = array_sum($ratios);
 
-        $remainder = $this;
+        [, $remainder] = $this->quotientAndRemainder($total);
 
-        foreach ($ratios as $ratio) {
-            $money = $this->multipliedBy($ratio)->quotient($total);
-            $remainder = $remainder->minus($money);
-            $monies[] = $money;
-        }
+        $toAllocate = $this->minus($remainder);
+
+        $monies = array_map(
+            fn (int $ratio) => $toAllocate->multipliedBy($ratio)->dividedBy($total),
+            $ratios,
+        );
 
         $monies[] = $remainder;
 
         return $monies;
     }
 
+    /**
+     * @param int[] $ratios
+     * @psalm-param list<int> $ratios
+     *
+     * @return int[]
+     * @psalm-return list<int>
+     */
+    private function simplifyRatios(array $ratios): array
+    {
+        $gcd = $this->gcdOfMultipleInt($ratios);
+
+        return array_map(fn (int $ratio) => intdiv($ratio, $gcd), $ratios);
+    }
+
+    /**
+     * @param int[] $values
+     *
+     * @psalm-param list<int> $values
+     */
+    private function gcdOfMultipleInt(array $values): int
+    {
+        $values = array_map(fn (int $value) => BigInteger::of($value), $values);
+
+        return $this->gcdOfMultipleBigInteger($values)->toInt();
+    }
+
+    /**
+     * @param BigInteger[] $values
+     *
+     * @psalm-param list<BigInteger> $values
+     */
+    private function gcdOfMultipleBigInteger(array $values): BigInteger
+    {
+        $count = count($values);
+
+        if ($count === 0) {
+            throw new InvalidArgumentException(__METHOD__ . ' requires at least one value.');
+        }
+
+        $result = $values[0];
+
+        for ($i = 1; $i < $count; $i++) {
+            $result = $result->gcd($values[$i]);
+
+            if ($result->isEqualTo(1)) {
+                return $result;
+            }
+        }
+
+        return $result;
+    }
+
     /**
      * Splits this Money into a number of parts.
      *

diff --git a/tests/MoneyTest.php b/tests/MoneyTest.php
@@ -431,10 +431,10 @@ public function providerAllocateWithRemainder() : array
             [['99.99', 'USD'], [100, 100], ['USD 49.99', 'USD 49.99', 'USD 0.01']],
             [[100, 'USD'], [30, 20, 40], ['USD 33.33', 'USD 22.22', 'USD 44.44', 'USD 0.01']],
             [[100, 'USD'], [30, 20, 40, 40], ['USD 23.07', 'USD 15.38', 'USD 30.76', 'USD 30.76', 'USD 0.03']],
-            [[100, 'CHF', new CashContext(5)], [1, 2, 3, 7], ['CHF 7.65', 'CHF 15.35', 'CHF 23.05', 'CHF 53.80', 'CHF 0.15']],
+            [[100, 'CHF', new CashContext(5)], [1, 2, 3, 7], ['CHF 7.65', 'CHF 15.30', 'CHF 22.95', 'CHF 53.55', 'CHF 0.55']],
             [['100.123', 'EUR', new AutoContext()], [2, 3, 1, 1], ['EUR 28.606', 'EUR 42.909', 'EUR 14.303', 'EUR 14.303', 'EUR 0.002']],
             [['0.02', 'EUR'], [1, 1, 1, 1], ['EUR 0.00', 'EUR 0.00', 'EUR 0.00', 'EUR 0.00', 'EUR 0.02']],
-            [['0.02', 'EUR'], [1, 1, 3, 1], ['EUR 0.00', 'EUR 0.00', 'EUR 0.01', 'EUR 0.00', 'EUR 0.01']],
+            [['0.02', 'EUR'], [1, 1, 3, 1], ['EUR 0.00', 'EUR 0.00', 'EUR 0.00', 'EUR 0.00', 'EUR 0.02']],
             [[-100, 'USD'], [30, 20, 40, 40], ['USD -23.07', 'USD -15.38', 'USD -30.76', 'USD -30.76', 'USD -0.03']],
             [['0.03', 'GBP'], [75, 25], ['GBP 0.00', 'GBP 0.00', 'GBP 0.03']],
         ];
@@ -513,7 +513,7 @@ public function providerSplitWithRemainder() : array
             [['99.99', 'USD'], 4, ['USD 24.99', 'USD 24.99', 'USD 24.99', 'USD 24.99', 'USD 0.03']],
             [[100, 'CHF', new CashContext(5)], 3, ['CHF 33.30', 'CHF 33.30', 'CHF 33.30', 'CHF 0.10']],
             [[100, 'CHF', new CashContext(5)], 7, ['CHF 14.25','CHF 14.25', 'CHF 14.25', 'CHF 14.25', 'CHF 14.25', 'CHF 14.25', 'CHF 14.25', 'CHF 0.25']],
-            [['100.123', 'EUR', new AutoContext()], 4, ['EUR 25.030', 'EUR 25.030', 'EUR 25.030', 'EUR 25.030', 'EUR 0.003']],
+            [['100.123', 'EUR', new AutoContext()], 4, ['EUR 25.03', 'EUR 25.03', 'EUR 25.03', 'EUR 25.03', 'EUR 0.003']],
             [['0.02', 'EUR'], 4, ['EUR 0.00', 'EUR 0.00', 'EUR 0.00', 'EUR 0.00', 'EUR 0.02']],
         ];
     }