brick / money

A money and currency library for PHP
MIT License
1.61k stars 96 forks source link

NumberFormatException on multiply operation #85

Closed tems99 closed 4 months ago

tems99 commented 4 months ago

Version: 0.9.0 I ran a phpunit test on the following code.

$money1 = Money::of(100, 'USD');
$money2 = Money::of(200, 'USD');
$money3 = $money1->multipliedBy($money2);

The above code throws an exception.

Brick\Math\Exception\NumberFormatException: The given value "USD 200.00" does not represent a valid number. /app/vendor/brick/math/src/Exception/NumberFormatException.php:14 /app/vendor/brick/math/src/BigNumber.php:118 /app/vendor/brick/math/src/BigNumber.php:61 /app/vendor/brick/math/src/BigRational.php:233 /app/vendor/brick/money/src/Money.php:405

Any ideas or am I doing something wrong?

tems99 commented 4 months ago

I read the documentation and it did not state that you could multiply with Money objects. Also, the signature for the multiplyBy function does not contain Money or AbstractMoney as its parameters. When debugging, if you pass in a Money object, it is accepted by the function as a string representation, which causes the error. I will be closing this because it is not an issue.

BenMorel commented 4 months ago

Hi, indeed, multiplying a Money with a Money does not make sense, as you found out, you need to multiply with a number!

I advise you to use strict_types, which would have caught the error early instead of attempting to cast the Money to string and use that as a number:

<?php

declare(strict_types=1);

require 'vendor/autoload.php';

use Brick\Money\Money;

$money1 = Money::of(100, 'USD');
$money2 = Money::of(200, 'USD');
$money3 = $money1->multipliedBy($money2);

PHP Fatal error: Uncaught TypeError: Brick\Money\Money::multipliedBy(): Argument #1 ($that) must be of type Brick\Math\BigNumber|string|int|float, Brick\Money\Money given