Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

[Saki Takamachi]

Hi, 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 I've thought twice about making classes final. The return type issue can be solved by specifying static, but as Barney mentioned, there is a problem with operator overloading, for example addition, where the class of the resulting object changes if the left and right operands are swapped. In other words, if that problem can be solved, it may be possible to make a class inheritable without making it final. The only solution I can think of at the moment is to impose the constraint that when computing operator overloading, if the operands are both objects, they must be of the exact same class. When calculating using a method, it is clear which object should be prioritized, so there is no risk of breaking down even if the method accepts objects of different classes as arguments. It would be possible to unify the behavior of the methods with operator overloading, but this would be undesirable because the arguments would be contravariant. I think my idea is somewhat better than using final classes in terms of freedom for the user. Whether it's a good API or not may require some further discussion. However, I think it is unlikely that a user will misunderstand and introduce a bug into their code because an error will occur before it will act as a bug. Alternatively, a user may misunderstand that a method can only receive objects of the same class as an argument, but this only restricts the degree of freedom and does not create a bug in itself. The only concern is that when do a calculation like `$numChild->add($num)`, users might misunderstand that it returns the class of the argument object. I'm a bit unsure if this could be a solution as there may still be something I've overlooked. Regards. Saki