Re: [RFC] [Discussion] Support object type in BCMath

Hi On 4/5/24 20:30, Jordan LeDoux wrote:> 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.

You are right of course. It should've read 10 USD.

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.

Opening up the class after-the-fact is always possible, should reasonable use-cases be proposed that would be enabled by it. Closing it down (i.e. marking it final), should it turn out that allowing to extend it was a bad choice, is not possible. Thus 'final' is the safe default choice.

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.

The operator overloads are already useless, because they can't usefully return the subclassed type even of the currencies would match for the money example.

    $fiveEuros = new Money(5, 'EUR');
    $tenEuros = new Money(10, 'EUR');

    $what = $fiveEuros + $tenEuros;

The only reasonable result for the implementation would be Number(15), because the addition operator would not know what to do with the extra fields. Likewise:

    $fiveMeters = new Distance(500);
    $oneMinute = new Time(60000);

    $what = $fiveMeters * $oneMinute;

is equally unsound, because 5 meters per minute is something different than 30000000, 300, 5, 0.08333333 or whatever the internal representation of those classes looks like. Numbers with units are not interchangeable with scalars. Generically speaking getting back the Number class when invoking any of the existing methods on an object of the child class drastically reduces the little bit of usefulness a child class might have.

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.

It's a reasonably common source of bugs. Here's several closely-related fixes: https://github.com/php/php-src/commit/93becab506ac05a7bddce1ceaacb53d6657180ce https://github.com/php/php-src/commit/a22558183309c05bb6bd7946ea1063143654f200 https://github.com/php/php-src/commit/85fbc6eaa6cd596f67d533091b50b2e2fcf9b601 I remember that other issues related to subclassing the date classes have arisen in the past. Best regards Tim Düsterhus