Skip to content

Commit

Permalink
money's internal amount should be a string
Browse files Browse the repository at this point in the history
  • Loading branch information
@frederikbosch
frederikbosch authored and Márk Sági-Kazár committed Jan 13, 2016
1 parent 6df55c5 commit 04490c1
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 61 deletions.
46 changes: 23 additions & 23 deletions src/Calculator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,79 +21,79 @@ public static function supported();
/**
* Compare a to b
*
* @param int|string $a
* @param int|string $b
* @param string $a
* @param string $b
* @return int
*/
public function compare($a, $b);

/**
* Add added to amount
*
* @param int|string $amount
* @param int|string $addend
* @return int|string
* @param string $amount
* @param string $addend
* @return string
*/
public function add($amount, $addend);

/**
* Subtract subtrahend from amount
* @param int|string $amount
* @param int|string $subtrahend
* @return int|string
* @param string $amount
* @param string $subtrahend
* @return string
*/
public function subtract($amount, $subtrahend);

/**
* Multiply amount with multiplier
*
* @param int|string $amount
* @param int|float $multiplier
* @return int|string
* @param string $amount
* @param int|float|string $multiplier
* @return string
*/
public function multiply($amount, $multiplier);

/**
* Divide amount with divisor
*
* @param int|string $amount
* @param int|float $divisor
* @return int|string
* @param string $amount
* @param int|float|string $divisor
* @return string
*/
public function divide($amount, $divisor);

/**
* Round number to following integer
*
* @param int|string|float $number
* @return int|string
* @param string $number
* @return string
*/
public function ceil($number);

/**
* Round number to preceding integer
*
* @param int|string|float $number
* @return int|string
* @param string $number
* @return string
*/
public function floor($number);

/**
* Round number, use rounding mode for tie-breaker
*
* @param int|string|float $number
* @param int|string $roundingMode
* @return int|string
* @param string $number
* @param int $roundingMode
* @return string
*/
public function round($number, $roundingMode);

/**
* Share amount among ratio / total portions
*
* @param int|string $amount
* @param string $amount
* @param int|float $ratio
* @param int|float $total
* @return int|string
* @return string
*/
public function share($amount, $ratio, $total);
}
12 changes: 6 additions & 6 deletions src/Calculator/PhpCalculator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function add($amount, $addend)

$this->assertInteger($result);

return $result;
return (string) $result;
}

/**
Expand All @@ -46,7 +46,7 @@ public function subtract($amount, $subtrahend)

$this->assertInteger($result);

return $result;
return (string) $result;
}

/**
Expand All @@ -58,7 +58,7 @@ public function multiply($amount, $multiplier)

$this->assertIntegerBounds($result);

return $result;
return (string) $result;
}

/**
Expand All @@ -70,7 +70,7 @@ public function divide($amount, $divisor)

$this->assertIntegerBounds($result);

return $result;
return (string) $result;
}

/**
Expand Down Expand Up @@ -134,7 +134,7 @@ private function castInteger($amount)
{
$this->assertIntegerBounds($amount);

return intval($amount);
return (string) intval($amount);
}

/**
Expand All @@ -144,7 +144,7 @@ private function castInteger($amount)
*/
private function assertInteger($amount)
{
if (!is_int($amount)) {
if (filter_var($amount, FILTER_VALIDATE_INT) === false) {
throw new \UnexpectedValueException('The result of arithmetic operation is not an integer');
}
}
Expand Down
18 changes: 9 additions & 9 deletions src/Money.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ final class Money implements JsonSerializable
];

/**
* @param int $amount Amount, expressed in the smallest units of $currency (eg cents)
* @param int|string $amount Amount, expressed in the smallest units of $currency (eg cents)
* @param Currency $currency
*
* @throws InvalidArgumentException If amount is not integer
Expand All @@ -58,7 +58,7 @@ public function __construct($amount, Currency $currency)
throw new InvalidArgumentException('Amount must be an integer');
}

$this->amount = $amount;
$this->amount = (string) $amount;
$this->currency = $currency;
}

Expand All @@ -84,7 +84,7 @@ public static function __callStatic($method, $arguments)
/**
* Returns a new Money instance based on the current one using the Currency.
*
* @param int $amount
* @param int|string $amount
*
* @return Money
*
Expand Down Expand Up @@ -196,7 +196,7 @@ public function lessThanOrEqual(Money $other)
/**
* Returns the value represented by this object.
*
* @return int
* @return string
*/
public function getAmount()
{
Expand Down Expand Up @@ -282,7 +282,7 @@ private function assertRoundingMode($roundingMode)
* Returns a new Money object that represents
* the multiplied value by the given factor.
*
* @param numeric $multiplier
* @param float|int|string $multiplier
* @param int $roundingMode
*
* @return Money
Expand Down Expand Up @@ -316,7 +316,7 @@ public function convert(Currency $targetCurrency, $conversionRate, $roundingMode
* Returns a new Money object that represents
* the divided value by the given factor.
*
* @param numeric $divisor
* @param float|int|string $divisor
* @param int $roundingMode
*
* @return Money
Expand Down Expand Up @@ -384,7 +384,7 @@ public function allocateTo($n)
* @param int|float $amount
* @param $rounding_mode
*
* @return int
* @return string
*/
private function round($amount, $rounding_mode)
{
Expand Down Expand Up @@ -434,7 +434,7 @@ public function isNegative()
*
* @param string $string
*
* @return int
* @return string
*
* @throws InvalidArgumentException If $string cannot be parsed
*/
Expand All @@ -455,7 +455,7 @@ public static function stringToUnits($string)
$units .= isset($matches['decimal1']) ? $matches['decimal1'] : '0';
$units .= isset($matches['decimal2']) ? $matches['decimal2'] : '0';

return (int) $units;
return (string) $units;
}

/**
Expand Down
7 changes: 6 additions & 1 deletion tests/MoneyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,12 @@ public function testEquality()
$m2 = new Money(100, new Currency('EUR'));
$m3 = new Money(100, new Currency('USD'));
$m4 = new Money(50, new Currency('EUR'));
$m5 = new Money('100', new Currency('EUR'));

$this->assertTrue($m1->equals($m2));
$this->assertFalse($m1->equals($m3));
$this->assertFalse($m1->equals($m4));
$this->assertTrue($m1->equals($m5));
}

public function testAddition()
Expand Down Expand Up @@ -296,8 +298,11 @@ public function testAllocationToInvalidTargets()
public function testComparators()
{
$this->assertTrue(Money::EUR(0)->isZero());
$this->assertTrue(Money::EUR('0')->isZero());
$this->assertTrue(Money::EUR(-1)->isNegative());
$this->assertTrue(Money::EUR('-1')->isNegative());
$this->assertTrue(Money::EUR(1)->isPositive());
$this->assertTrue(Money::EUR('1')->isPositive());
$this->assertFalse(Money::EUR(1)->isZero());
$this->assertFalse(Money::EUR(1)->isNegative());
$this->assertFalse(Money::EUR(-1)->isPositive());
Expand Down Expand Up @@ -347,7 +352,7 @@ public function testCannotConvertStringToUnits()
public function testJsonEncoding()
{
$this->assertEquals(
'{"amount":350,"currency":"USD"}',
'{"amount":"350","currency":"USD"}',
json_encode(Money::USD(350))
);
}
Expand Down
44 changes: 22 additions & 22 deletions tests/PhpCalculatorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,68 +10,68 @@ class PhpCalculatorTest extends \PHPUnit_Framework_TestCase
public function testCompare()
{
$calculator = new PhpCalculator();
$this->assertEquals(1, $calculator->compare(2, 1));
$this->assertEquals(-1, $calculator->compare(1, 2));
$this->assertEquals(0, $calculator->compare(1, 1));
$this->assertSame(1, $calculator->compare(2, 1));
$this->assertSame(-1, $calculator->compare(1, 2));
$this->assertSame(0, $calculator->compare(1, 1));
}

public function testAdd()
{
$calculator = new PhpCalculator();
$this->assertEquals(2, $calculator->add(1, 1));
$this->assertSame('2', $calculator->add(1, 1));
}

public function testSubtract()
{
$calculator = new PhpCalculator();
$this->assertEquals(1, $calculator->subtract(2, 1));
$this->assertSame('1', $calculator->subtract(2, 1));
}

public function testMultiply()
{
$calculator = new PhpCalculator();
$this->assertEquals('1.5', $calculator->multiply(1, 1.5));
$this->assertSame('1.5', $calculator->multiply(1, 1.5));
}

public function testDivide()
{
$calculator = new PhpCalculator();
$this->assertEquals('1.5', $calculator->divide(3, 2));
$this->assertSame('1.5', $calculator->divide(3, 2));
}

public function testCeil()
{
$calculator = new PhpCalculator();
$this->assertEquals('2', $calculator->ceil(1.2));
$this->assertSame('2', $calculator->ceil(1.2));
}

public function testFloor()
{
$calculator = new PhpCalculator();
$this->assertEquals('2', $calculator->floor(2.7));
$this->assertSame('2', $calculator->floor(2.7));
}

public function testRound()
{
$calculator = new PhpCalculator();
$this->assertEquals('3', $calculator->round(2.6, Money::ROUND_HALF_EVEN));
$this->assertEquals('2', $calculator->round(2.5, Money::ROUND_HALF_EVEN));
$this->assertEquals('2', $calculator->round(2.1, Money::ROUND_HALF_ODD));
$this->assertEquals('3', $calculator->round(2.5, Money::ROUND_HALF_ODD));
$this->assertEquals('2', $calculator->round(2.5, Money::ROUND_HALF_DOWN));
$this->assertEquals('3', $calculator->round(2.6, Money::ROUND_HALF_DOWN));
$this->assertEquals('2', $calculator->round(2.2, Money::ROUND_HALF_UP));
$this->assertEquals('3', $calculator->round(2.5, Money::ROUND_HALF_UP));
$this->assertEquals('2', $calculator->round(2, Money::ROUND_HALF_UP));
$this->assertEquals('2', $calculator->round(2, Money::ROUND_HALF_DOWN));
$this->assertEquals('2', $calculator->round(2, Money::ROUND_HALF_EVEN));
$this->assertEquals('2', $calculator->round(2, Money::ROUND_HALF_ODD));
$this->assertSame('3', $calculator->round(2.6, Money::ROUND_HALF_EVEN));
$this->assertSame('2', $calculator->round(2.5, Money::ROUND_HALF_EVEN));
$this->assertSame('2', $calculator->round(2.1, Money::ROUND_HALF_ODD));
$this->assertSame('3', $calculator->round(2.5, Money::ROUND_HALF_ODD));
$this->assertSame('2', $calculator->round(2.5, Money::ROUND_HALF_DOWN));
$this->assertSame('3', $calculator->round(2.6, Money::ROUND_HALF_DOWN));
$this->assertSame('2', $calculator->round(2.2, Money::ROUND_HALF_UP));
$this->assertSame('3', $calculator->round(2.5, Money::ROUND_HALF_UP));
$this->assertSame('2', $calculator->round(2, Money::ROUND_HALF_UP));
$this->assertSame('2', $calculator->round(2, Money::ROUND_HALF_DOWN));
$this->assertSame('2', $calculator->round(2, Money::ROUND_HALF_EVEN));
$this->assertSame('2', $calculator->round(2, Money::ROUND_HALF_ODD));
}

public function testShare()
{
$calculator = new PhpCalculator();
$this->assertEquals('5', $calculator->share(10, 2, 4));
$this->assertSame('5', $calculator->share(10, 2, 4));
}

/**
Expand Down

3 comments on commit 04490c1

@hamdrew
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why the numeric amount should be represented as a string instead of an integer? The PR this was included on had no explanation and the commit message is not descriptive.

@frederikbosch
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we support unlimited integers nowadays. So that means that integers higher than PHP_INT_MAX can be used (through BC and GMP extrnsions). In order to do so, we can no longer use integers but have to use strings.

@hamdrew
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frederikbosch That makes sense, thanks for the explanation.

Please sign in to comment.