Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122766
Content-Type: text/plain;
	charset=us-ascii
Precedence: bulk
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.700.6\))
Subject: Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath
In-Reply-To: <bbcff629-4dd6-bc50-ac9f-641849dc218a@php.net>
Date: Wed, 27 Mar 2024 09:03:24 +0900
Cc: php internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <68CF373E-6ABF-4471-8992-813B4BA1B508@sakiot.com>
References: <4F094EDA-5058-407D-AF39-06FD934FDE1F@sakiot.com>
 <bbcff629-4dd6-bc50-ac9f-641849dc218a@php.net>
To: Derick Rethans <derick@php.net>
From: saki@sakiot.com (Saki Takamachi)

Hi Derick,

> - You've picked as class name "BcNum". Following=20
>  our naming guidelines, that probably should be \BCMath\Num (or=20
>  \BC\Num, but that is less descriptive):=20
>  =
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#=
namespaces-in-extensions
>=20
>  The reason it *should* have "BC" is that it comes from "Basic=20
>  Calculator" (https://www.php.net/manual/en/book.bc.php#118203)


I re-read the namespace RFC again. I also re-read the RFC regarding =
class naming conventions.
https://wiki.php.net/rfc/namespaces_in_bundled_extensions
https://wiki.php.net/rfc/class-naming

There's no need for the namespace to follow class naming conventions, =
but the acronym doesn't seem to need to be pascal-case anyway (I =
remembered it incorrectly). However, the RFC states that the extension's =
namespace must match the extension name, so it seems correct in this =
case for the namespace to be `BCMath`.

And indeed, looking at the example in the namespace RFC, `BCMath\Number` =
might be appropriate in this case (I think I was sleepy yesterday).

I changed `BcNum` to `BCMath\Number` in my RFC.


> - Should ->value rather be ->toString() ? ->value alone doesn't really=20=

>  say much. I'm on the fence here though, as there is already=20
>  (internally) a ->__toString() method to make the (string) cast work.

What is the main difference between getting a read-only property with =
`->value` and getting the value using a method?


> - Would it make sense to have "floor" and "ceil" to also have a scale,=20=

>  or precision? Or would developers instead have to use "round" in that=20=

>  case?

> - Which rounding modes are supported with "round", the same ones as =
the=20
>  normal round() function?

`bcfloor` and `bcceil` originally have no scale specification. This is =
because the result is always a string representing an integer value. And =
since the supported round-mode is the same as standard-round, =
`ROUND_FLOOR` and `ROUND_CEILING` are also supported.
Therefore, if want to obtain floor or ceil behavior with a specified =
scale, I recommend specifying the mode as round.


> - In this example, what would $result->scale show? (Perhaps add that =
to=20
>  the example?):
>=20
> <?php
> $num =3D new BcNum('1.23', 2);
> $result =3D $num + '1.23456';
> $result->value; // '2.46456'
> $result->scale; // ??

In this case, `$result->scale` will be `'5'`. I added this to the RFC.


> - Exceptions
>=20
>  The RFC does not mention which exceptions can be thrown. Is it just=20=

>  the one? It might be beneficial to *do* have a new exception=20
>  hierarchy.



As far as I know right now, following exceptions can be thrown:

- Value error when a string that is invalid as a number is used in a =
constructor, calculation method, or operation
- Divide by 0 error (include Modulo by zero)

I was thinking that it would be a bad idea to increase the number of =
classes without thinking, and was planning to use general exceptions, =
but would it be better to use dedicated exceptions?
By the way, generally when implementing such exceptions in userland, =
value errors and divide-by-zero errors are probably defined as separate =
classes, but should they be separated in this case?

Regards.

Saki