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
