-
Notifications
You must be signed in to change notification settings - Fork 441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/money refactor #55
Changes from 8 commits
b89c34b
56c3472
310cb6f
7dc8368
4d8294c
4b895f9
f848755
d16e01d
42c67ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,9 @@ | |
namespace Money; | ||
|
||
use InvalidArgumentException; | ||
use OverflowException; | ||
use UnderflowException; | ||
use UnexpectedValueException; | ||
|
||
/** | ||
* Money Value Object | ||
|
@@ -46,8 +49,9 @@ class Money | |
public function __construct($amount, Currency $currency) | ||
{ | ||
if (!is_int($amount)) { | ||
throw new InvalidArgumentException("The first parameter of Money must be an integer. It's the amount, expressed in the smallest units of currency (eg cents)"); | ||
throw new InvalidArgumentException('Amount must be an integer'); | ||
} | ||
|
||
$this->amount = $amount; | ||
$this->currency = $currency; | ||
} | ||
|
@@ -101,7 +105,7 @@ public function isSameCurrency(Money $other) | |
private function assertSameCurrency(Money $other) | ||
{ | ||
if (!$this->isSameCurrency($other)) { | ||
throw new InvalidArgumentException('Different currencies'); | ||
throw new InvalidArgumentException('Currencies must be identical'); | ||
} | ||
} | ||
|
||
|
@@ -114,9 +118,7 @@ private function assertSameCurrency(Money $other) | |
*/ | ||
public function equals(Money $other) | ||
{ | ||
return | ||
$this->isSameCurrency($other) | ||
&& $this->amount == $other->amount; | ||
return $this->isSameCurrency($other) && $this->amount == $other->amount; | ||
} | ||
|
||
/** | ||
|
@@ -131,6 +133,7 @@ public function equals(Money $other) | |
public function compare(Money $other) | ||
{ | ||
$this->assertSameCurrency($other); | ||
|
||
if ($this->amount < $other->amount) { | ||
return -1; | ||
} elseif ($this->amount == $other->amount) { | ||
|
@@ -196,6 +199,18 @@ public function getCurrency() | |
return $this->currency; | ||
} | ||
|
||
/** | ||
* Asserts that integer remains integer after arithmetic operations | ||
* | ||
* @param numeric $amount | ||
*/ | ||
public function assertInteger($amount) | ||
{ | ||
if (!is_int($amount)) { | ||
throw new UnexpectedValueException('The result of arithmetic operation is not an integer'); | ||
} | ||
} | ||
|
||
/** | ||
* Returns a new Money object that represents | ||
* the sum of this and an other Money object | ||
|
@@ -208,7 +223,11 @@ public function add(Money $addend) | |
{ | ||
$this->assertSameCurrency($addend); | ||
|
||
return $this->newInstance($this->amount + $addend->amount); | ||
$amount = $this->amount + $addend->amount; | ||
|
||
$this->assertInteger($amount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can't possibly fail, the sum of two integers is an integer. |
||
|
||
return $this->newInstance($amount); | ||
} | ||
|
||
/** | ||
|
@@ -223,7 +242,11 @@ public function subtract(Money $subtrahend) | |
{ | ||
$this->assertSameCurrency($subtrahend); | ||
|
||
return $this->newInstance($this->amount - $subtrahend->amount); | ||
$amount = $this->amount - $subtrahend->amount; | ||
|
||
$this->assertInteger($amount); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can't possibly fail, the subtraction of two integers is an integer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not in the case of an overflow. Php overflows to float. On Tuesday, 2 September 2014, Mathias Verraes [email protected]
|
||
|
||
return $this->newInstance($amount); | ||
} | ||
|
||
/** | ||
|
@@ -238,6 +261,38 @@ private function assertOperand($operand) | |
} | ||
} | ||
|
||
/** | ||
* Asserts that an integer value didn't become something else | ||
* (after some arithmetic operation) | ||
* | ||
* @param numeric $amount | ||
* | ||
* @throws OverflowException If integer overflow occured | ||
* @throws UnderflowException If integer underflow occured | ||
*/ | ||
private function assertIntegerBounds($amount) | ||
{ | ||
if ($amount > PHP_INT_MAX) { | ||
throw new OverflowException; | ||
} elseif ($amount < ~PHP_INT_MAX) { | ||
throw new UnderflowException; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really see the point. If you have 92233720368547758.07 EUR/USD/... then you probably didn't make that using this library :-) Oh well, happy to merge it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Haha, yeah I see what you mean. The context where it has been useful for me On Tuesday, 2 September 2014, Mathias Verraes [email protected]
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The max/avarage amount is dependent of the currency. So in case of high inflation, it easily reaches the maximum integer value. Not speaking of 32bit integer. |
||
|
||
/** | ||
* Casts an amount to integer ensuring that an overflow/underflow did not occur | ||
* | ||
* @param numeric $amount | ||
* | ||
* @return integer | ||
*/ | ||
private function castInteger($amount) | ||
{ | ||
$this->assertIntegerBounds($amount); | ||
|
||
return intval($amount); | ||
} | ||
|
||
/** | ||
* Returns a new Money object that represents | ||
* the multiplied value by the given factor | ||
|
@@ -255,7 +310,9 @@ public function multiply($multiplier, $rounding_mode = self::ROUND_HALF_UP) | |
$rounding_mode = new RoundingMode($rounding_mode); | ||
} | ||
|
||
$product = (int) round($this->amount * $multiplier, 0, $rounding_mode->getRoundingMode()); | ||
$product = round($this->amount * $multiplier, 0, $rounding_mode->getRoundingMode()); | ||
|
||
$product = $this->castInteger($product); | ||
|
||
return $this->newInstance($product); | ||
} | ||
|
@@ -277,7 +334,9 @@ public function divide($divisor, $rounding_mode = self::ROUND_HALF_UP) | |
$rounding_mode = new RoundingMode($rounding_mode); | ||
} | ||
|
||
$quotient = (int) round($this->amount / $divisor, 0, $rounding_mode->getRoundingMode()); | ||
$quotient = round($this->amount / $divisor, 0, $rounding_mode->getRoundingMode()); | ||
|
||
$quotient = $this->castInteger($quotient); | ||
|
||
return $this->newInstance($quotient); | ||
} | ||
|
@@ -287,7 +346,7 @@ public function divide($divisor, $rounding_mode = self::ROUND_HALF_UP) | |
* | ||
* @param array $ratios | ||
* | ||
* @return Money | ||
* @return Money[] | ||
*/ | ||
public function allocate(array $ratios) | ||
{ | ||
|
@@ -296,10 +355,11 @@ public function allocate(array $ratios) | |
$total = array_sum($ratios); | ||
|
||
foreach ($ratios as $ratio) { | ||
$share = (int) floor($this->amount * $ratio / $total); | ||
$share = $this->castInteger($this->amount * $ratio / $total); | ||
$results[] = $this->newInstance($share); | ||
$remainder -= $share; | ||
} | ||
|
||
for ($i = 0; $remainder > 0; $i++) { | ||
$results[$i]->amount++; | ||
$remainder--; | ||
|
@@ -351,8 +411,9 @@ public static function stringToUnits($string) | |
{ | ||
//@todo extend the regular expression with grouping characters and eventually currencies | ||
if (!preg_match("/(-)?(\d+)([.,])?(\d)?(\d)?/", $string, $matches)) { | ||
throw new InvalidArgumentException("The value could not be parsed as money"); | ||
throw new InvalidArgumentException('The value could not be parsed as money'); | ||
} | ||
|
||
$units = $matches[1] == "-" ? "-" : ""; | ||
$units .= $matches[2]; | ||
$units .= isset($matches[4]) ? $matches[4] : "0"; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why public?