Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122979
Precedence: bulk
MIME-Version: 1.0
References: <4F094EDA-5058-407D-AF39-06FD934FDE1F@sakiot.com>
 <bbcff629-4dd6-bc50-ac9f-641849dc218a@php.net> <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>
In-Reply-To: <8035a2b2-f8dc-4a25-98eb-1d19c986bc75@bastelstu.be>
Date: Fri, 5 Apr 2024 11:30:24 -0700
Message-ID: <CAMrTa2Geo1XHFxfa1Mj6tXcYC-gVwQeUK+Wk0h11d1fELUyZJQ@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>
Cc: Saki Takamachi <saki@sakiot.com>, Lynn <kjarli@gmail.com>, 
	Aleksander Machniak <alec@alec.pl>, php internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000038831506155da75c"
From: jordan.ledoux@gmail.com (Jordan LeDoux)

--00000000000038831506155da75c
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 5, 2024 at 2:48=E2=80=AFAM Tim D=C3=BCsterhus <tim@bastelstu.be=
> wrote:

> Hi
>
> Your `Money` example would allow for unsound and/or non-sense behavior,
> such as:
>
>      $fiveEuros =3D new Money(5, 'EUR');
>      $tenDollars =3D new Money(10, 'EUR');
>
>      $what =3D $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 =3D $val1->getCurrency();
$val2Currency =3D $val2->getCurrency();
$val1 =3D $val1->convertToCommonCurrency();
$val2 =3D $val2->convertToCommonCurrency();
// perform the necessary calculations using operators
$answer =3D $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

--00000000000038831506155da75c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Fri, Apr 5, 2024 at 2:48=E2=80=AFA=
M Tim D=C3=BCsterhus &lt;<a href=3D"mailto:tim@bastelstu.be">tim@bastelstu.=
be</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margi=
n:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex=
">Hi<br>
<br>
Your `Money` example would allow for unsound and/or non-sense behavior, <br=
>
such as:<br>
<br>
=C2=A0 =C2=A0 =C2=A0$fiveEuros =3D new Money(5, &#39;EUR&#39;);<br>
=C2=A0 =C2=A0 =C2=A0$tenDollars =3D new Money(10, &#39;EUR&#39;);<br>
<br>
=C2=A0 =C2=A0 =C2=A0$what =3D $fiveEuros + $tenDollars;<br>
<br>
What would you expect to be in $what? A `BcMath\Number(15)`?<br>
<br>
----------------<br>
<br>
The BcMath\Number class *should* absolutely be final, as a number is a <br>
number is a number. Allowing extension just to be able to write <br>
$number-&gt;isPrime() instead of isPrime($number) will allow for very <br>
confusing code, such as the example above, with no way to work around it <b=
r>
in userland. It also makes interoperability between two different <br>
libraries that expect their own extensions to work very painful.<br>
<br>
Consider the following example:<br>
<br>
=C2=A0 =C2=A0 =C2=A0class PrimalityTestingNumber extends Number {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function isPrime(): bool { }<br>
=C2=A0 =C2=A0 =C2=A0}<br>
<br>
=C2=A0 =C2=A0 =C2=A0class ParityTestingNumber extends Number {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function isEven(): bool { }<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0public function isOdd(): bool { }<br>
=C2=A0 =C2=A0 =C2=A0}<br>
<br>
If I now want to create a function to check whether a number is an even <br=
>
prime, I need to do something like this:<br>
<br>
=C2=A0 =C2=A0 =C2=A0function isEvenPrime(Number $n) {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return (new PrimalityTestingNumber($n))-&=
gt;isPrime() &amp;&amp; (new <br>
ParityTestingNumber($n))-&gt;isEven();<br>
=C2=A0 =C2=A0 =C2=A0}<br>
<br>
This use case would be much better solved in a generic way using <br>
something like this &quot;Extension Methods&quot; proposal: <br>
<a href=3D"https://externals.io/message/118395#118395" rel=3D"noreferrer" t=
arget=3D"_blank">https://externals.io/message/118395#118395</a><br>
<br></blockquote><div><br></div><div>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.</div><div><br></div><div>As I said, you can=
 accomplish the same through composition. For a money class, you&#39;d like=
ly need an extended method that checks the currency first, and then makes a=
 call to the actual calculation method, which is essentially what the compo=
sing class would do as well, something like `addCurrency()`.</div><div><br>=
</div><div>Frankly, I don&#39;t think that making it final actually makes t=
he API more resistant to errors, but it&#39;s also not something I care abo=
ut enough to really fight for it on this RFC. I think it&#39;s the wrong ch=
oice, 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 f=
ind myself extremely hesitant to suggest that we have thought of and consid=
ered all the various use cases that developers might have, or how they migh=
t be impacted by making the entire class final.</div><div><br></div><div>Ma=
king it final will not reduce errors in my opinion, it will just make inter=
nals feel like those errors are less &quot;our fault&quot;. A composed clas=
s does not somehow prevent the accidental error of mixing currencies, it ju=
st moves where that error would occur. Forcing composition drastically redu=
ces the usability of the operator overloads, which I am opposed to, and I d=
o 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 an=
y kind of extensions either.</div><div><br></div><div>I mentioned that I ha=
ve a library that adds arbitrary precision functions for trigonometric func=
tions (among others), to extend BCMath (and ext-decimal). A library that is=
 solely focused on additional arbitrary precision math features should be t=
he first in line to use this RFC, but I can tell you that if this class is =
final, I won&#39;t even update my library to use it at all, because there w=
ill be no point. The reason I did not mention MY use case is because I don&=
#39;t think many PHP developers are out there maintaining their own complic=
ated trigonometric extensions to BCMath, so I don&#39;t think it&#39;s a go=
od example of a common use case. <br></div><div><br></div><div>Making it fi=
nal also breaks with how other internally provided classes have been done i=
n the past, many with no discernable issues. I do not see any mailing list =
discussions about the problems with DateTime being open for extension.</div=
><div><br></div><div>If you want an actual answer about how a Money class w=
ould actually work in practice, it would likely be something like this:</di=
v><div><br></div><div>```</div><div>// $val1 and $val2 are instances of the=
 Money class with unknown currencies</div><div>$val1Currency =3D $val1-&gt;=
getCurrency();</div><div>$val2Currency =3D $val2-&gt;getCurrency();</div><d=
iv>$val1 =3D $val1-&gt;convertToCommonCurrency();</div><div>$val2 =3D $val2=
-&gt;convertToCommonCurrency();</div><div>// perform the necessary calculat=
ions using operators</div><div>$answer =3D $answer-&gt;convertToCurrency($u=
serCurrency);</div><div>```</div><div><br></div><div>I would expect that mo=
st applications dealing with money are converting to a common calculation c=
urrency prior to actually doing any calculations, instead of relying on the=
 conversion magically happening inside their calculation methods.</div><div=
><br></div><div>So, your argument leaves me entirely unmoved. However, as I=
 said, while this makes the RFC essentially useless to me, I don&#39;t thin=
k it&#39;s worth rejecting the RFC over, so I&#39;ll leave it at that.</div=
><div><br></div><div>Jordan<br></div></div></div>

--00000000000038831506155da75c--