On Fri, Apr 5, 2024 at 2:48 AM Tim Düsterhus <t...@bastelstu.be> wrote:
> Hi
>
> Your `Money` example would allow for unsound and/or non-sense behavior,
> such as:
>
> $fiveEuros = new Money(5, 'EUR');
> $tenDollars = new Money(10, 'EUR');
>
> $what = $fiveEuros + $tenDollars;
>
> What would you expect to be in $what? A `BcMath\Number(15)`?
>
> ----------------
>
> The BcMath\Number class *should* absolutely be final, as a number is a
> number is a number. Allowing extension just to be able to write
> $number->isPrime() instead of isPrime($number) will allow for very
> confusing code, such as the example above, with no way to work around it
> in userland. It also makes interoperability between two different
> libraries that expect their own extensions to work very painful.
>
> Consider the following example:
>
> class PrimalityTestingNumber extends Number {
> public function isPrime(): bool { }
> }
>
> class ParityTestingNumber extends Number {
> public function isEven(): bool { }
> public function isOdd(): bool { }
> }
>
> If I now want to create a function to check whether a number is an even
> prime, I need to do something like this:
>
> function isEvenPrime(Number $n) {
> return (new PrimalityTestingNumber($n))->isPrime() && (new
> ParityTestingNumber($n))->isEven();
> }
>
> This use case would be much better solved in a generic way using
> something like this "Extension Methods" proposal:
> https://externals.io/message/118395#118395
>
>
Well, since they are both in Euros, I would expect 15, but I expect that
was a typo considering the variable name and it was meant to be 10 USD.
As I said, you can accomplish the same through composition. For a money
class, you'd likely need an extended method that checks the currency first,
and then makes a call to the actual calculation method, which is
essentially what the composing class would do as well, something like
`addCurrency()`.
Frankly, I don't think that making it final actually makes the API more
resistant to errors, but it's also not something I care about enough to
really fight for it on this RFC. I think it's the wrong choice, because the
one example that I pulled up in this discussion does not constitute the
entire breadth of use cases for this type of object, and I find myself
extremely hesitant to suggest that we have thought of and considered all
the various use cases that developers might have, or how they might be
impacted by making the entire class final.
Making it final will not reduce errors in my opinion, it will just make
internals feel like those errors are less "our fault". A composed class
does not somehow prevent the accidental error of mixing currencies, it just
moves where that error would occur. Forcing composition drastically reduces
the usability of the operator overloads, which I am opposed to, and I do
not believe that this is being given the same kind of consideration. Once
the class is forced to be composed, it can no longer be used as part of any
kind of extensions either.
I mentioned that I have a library that adds arbitrary precision functions
for trigonometric functions (among others), to extend BCMath (and
ext-decimal). A library that is solely focused on additional arbitrary
precision math features should be the first in line to use this RFC, but I
can tell you that if this class is final, I won't even update my library to
use it at all, because there will be no point. The reason I did not mention
MY use case is because I don't think many PHP developers are out there
maintaining their own complicated trigonometric extensions to BCMath, so I
don't think it's a good example of a common use case.
Making it final also breaks with how other internally provided classes have
been done in the past, many with no discernable issues. I do not see any
mailing list discussions about the problems with DateTime being open for
extension.
If you want an actual answer about how a Money class would actually work in
practice, it would likely be something like this:
```
// $val1 and $val2 are instances of the Money class with unknown currencies
$val1Currency = $val1->getCurrency();
$val2Currency = $val2->getCurrency();
$val1 = $val1->convertToCommonCurrency();
$val2 = $val2->convertToCommonCurrency();
// perform the necessary calculations using operators
$answer = $answer->convertToCurrency($userCurrency);
```
I would expect that most applications dealing with money are converting to
a common calculation currency prior to actually doing any calculations,
instead of relying on the conversion magically happening inside their
calculation methods.
So, your argument leaves me entirely unmoved. However, as I said, while
this makes the RFC essentially useless to me, I don't think it's worth
rejecting the RFC over, so I'll leave it at that.
Jordan
