Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122983
Message-ID: <7fbce213-466f-4ce9-8c89-4dc9a5ef526b@bastelstu.be>
Date: Fri, 5 Apr 2024 21:52:20 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath
To: Jordan LeDoux <jordan.ledoux@gmail.com>
Cc: Saki Takamachi <saki@sakiot.com>, Lynn <kjarli@gmail.com>,
 Aleksander Machniak <alec@alec.pl>, php internals <internals@lists.php.net>
References: <4F094EDA-5058-407D-AF39-06FD934FDE1F@sakiot.com>
 <68CF373E-6ABF-4471-8992-813B4BA1B508@sakiot.com>
 <904197f4-afb5-401e-9e17-7a655c5449d0@alec.pl>
 <CAMrTa2EK6ymBQuxeFj7tMV0M2crqCCCR=mLN-FSfuCgrQHEZ6A@mail.gmail.com>
 <AB663F3F-A888-45F7-B19A-00530A4EDB0C@sakiot.com>
 <CAMrTa2HOEUWQcPdZE3FZ9JWCS1DWuRTNhhfqGGZOmr=v1Mctdw@mail.gmail.com>
 <CANS-=pfVuKKfAiu0LUJTrEPnSRaa_nHwJCgMvSORruAwHMsvQg@mail.gmail.com>
 <CAMrTa2HXD+62TKbaaA-v-mct5zdR5ortUAbtG1R1D22amAgpNA@mail.gmail.com>
 <CAMrTa2GaK3AUp0WodhNO8RdvoLbbiCk14Gs2oWF6vEs_+qMYNg@mail.gmail.com>
 <655FEA80-9AB4-4AAD-A310-70ED968C97A2@sakiot.com>
 <CAMrTa2Fh_-M_Kezi7s_4P4+a+dTyKq=RExd6ARPRDqZYS175kA@mail.gmail.com>
 <CAMrTa2FrkvNPK6kETNJFNNDn0mL5dxkV=PmdxsysruhhjY=Uig@mail.gmail.com>
 <8FB87901-02D7-4934-9119-55B21CEDDA9D@sakiot.com>
 <CAMrTa2EJf2NK2D3J=q7xkAVuNKsjEBDVv+RDmxp_g9DSsjRgwg@mail.gmail.com>
 <8035a2b2-f8dc-4a25-98eb-1d19c986bc75@bastelstu.be>
 <CAMrTa2Geo1XHFxfa1Mj6tXcYC-gVwQeUK+Wk0h11d1fELUyZJQ@mail.gmail.com>
Content-Language: en-US
In-Reply-To: <CAMrTa2Geo1XHFxfa1Mj6tXcYC-gVwQeUK+Wk0h11d1fELUyZJQ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
From: tim@bastelstu.be (=?UTF-8?Q?Tim_D=C3=BCsterhus?=)

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