Skip to content

Commit

Permalink
Fix BigNumber::sum() when calling method on BigNumber itself with mix…
Browse files Browse the repository at this point in the history
…ed types
  • Loading branch information
BenMorel committed Apr 11, 2019
1 parent fc1ae88 commit 44d14bb
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 10 deletions.
48 changes: 38 additions & 10 deletions src/BigNumber.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,15 +194,7 @@ public static function max(...$values) : BigNumber
*/
public static function sum(...$values) : BigNumber
{
/**
* Note: we'd benefit from a common interface BigNumber::plus(), but right now we can't because PHP does not
* support covariant return types, so that abstract BigNumber::plus() returns BigNumber, while
* BigInteger::plus() returns BigInteger. This will likely be possible in PHP 7.4:
*
* https://wiki.php.net/rfc/covariant-returns-and-contravariant-parameters
*
* @var BigInteger|BigDecimal|BigRational|null $sum
*/
/** @var BigNumber|null $sum */
$sum = null;

foreach ($values as $value) {
Expand All @@ -211,7 +203,7 @@ public static function sum(...$values) : BigNumber
if ($sum === null) {
$sum = $value;
} else {
$sum = $sum->plus($value);
$sum = self::add($sum, $value);
}
}

Expand All @@ -222,6 +214,42 @@ public static function sum(...$values) : BigNumber
return $sum;
}

/**
* Adds two BigNumber instances in the correct order to avoid a RoundingNecessaryException.
*
* @todo This could be better resolved by creating an abstract protected method in BigNumber, and leaving to
* concrete classes the responsibility to perform the addition themselves or delegate it to the given number,
* depending on their ability to perform the operation. This will also require a version bump because we're
* potentially breaking custom BigNumber implementations (if any...)
*
* @param BigNumber $a
* @param BigNumber $b
*
* @return BigNumber
*/
private static function add(BigNumber $a, BigNumber $b) : BigNumber
{
if ($a instanceof BigRational) {
return $a->plus($b);
}

if ($b instanceof BigRational) {
return $b->plus($a);
}

if ($a instanceof BigDecimal) {
return $a->plus($b);
}

if ($b instanceof BigDecimal) {
return $b->plus($a);
}

/** @var BigInteger $a */

return $a->plus($b);
}

/**
* Removes optional leading zeros and + sign from the given number.
*
Expand Down
54 changes: 54 additions & 0 deletions tests/BigNumberTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php

namespace Brick\Math\Tests;

use Brick\Math\BigDecimal;
use Brick\Math\BigInteger;
use Brick\Math\BigNumber;
use Brick\Math\BigRational;

/**
* Unit tests for class BigNumber.
*
* Most of the tests are performed in concrete classes.
* Only static methods that can be called on BigNumber itself may justify tests here.
*/
class BigNumberTest extends AbstractTestCase
{
/**
* @dataProvider providerSum
*
* @param array $values The values to add.
* @param string $expectedClass The expected class name.
* @param string $expectedSum The expected sum.
*/
public function testSum(array $values, string $expectedClass, string $expectedSum)
{
$sum = BigNumber::sum(...$values);

$this->assertSame($expectedClass, get_class($sum));
$this->assertSame($expectedSum, (string) $sum);
}

/**
* @return array
*/
public function providerSum()
{
return [
[[-1], BigInteger::class, '-1'],
[[-1, '99'], BigInteger::class, '98'],
[[-1, '99', '-0.7'], BigDecimal::class, '97.3'],
[[-1, '99', '-0.7', '3/2'], BigRational::class, '1976/20'],
[[-1, '3/2'], BigRational::class, '1/2'],
[[-0.5], BigDecimal::class, '-0.5'],
[[-0.5, 1], BigDecimal::class, '0.5'],
[[-0.5, 1, '0.7'], BigDecimal::class, '1.2'],
[[-0.5, 1, '0.7', '47/7'], BigRational::class, '554/70'],
[['-1/9'], BigRational::class, '-1/9'],
[['-1/9', 123], BigRational::class, '1106/9'],
[['-1/9', 123, '8349.3771'], BigRational::class, '762503939/90000'],
[['-1/9', '8349.3771', 123], BigRational::class, '762503939/90000']
];
}
}

0 comments on commit 44d14bb

Please sign in to comment.