Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:123067
Message-ID: <cb6bb26f-8db1-4a9a-851a-bc6ed7c43bfd@bastelstu.be>
Date: Tue, 9 Apr 2024 21:21:27 +0200
Precedence: bulk
MIME-Version: 1.0
Subject: Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath
To: Saki Takamachi <saki@sakiot.com>
Cc: internals@lists.php.net
References: <D1E3EBAD-A991-4591-B7DE-CF414593E64B@sakiot.com>
 <FB449C0E-1D83-476F-889D-19A6ABDBD38E@sakiot.com>
 <FF0FFBCE-7F03-44A2-B7AE-D3F958135ACB@sakiot.com>
 <b232c9f1-aad9-4ef3-8f23-d0cb2deb0baf@rwec.co.uk>
 <B6391516-7680-4B7C-BFBA-D5DEE274F030@sakiot.com>
 <959ccc6e-1d5c-48c0-9357-80e38becd1c5@bastelstu.be>
 <B7F2D363-BDCC-4AEA-A1AE-782C14DAD37A@sakiot.com>
Content-Language: en-US
In-Reply-To: <B7F2D363-BDCC-4AEA-A1AE-782C14DAD37A@sakiot.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/8/24 02:08, Saki Takamachi wrote:
>> - The full “stub” should probably include an explicit "implements Stringable" for clarity.
> 
> Agree. I describe it explicitly during implementation.
> 

I don't see a change made, so perhaps I was unclear in what I said. What 
I meant was: Please add a full stub example including all the 
interfaces, properties, and methods. It's currently split across 
multiple code blocks and the interfaces are missing entirely.

Example:

     final readonly class Number implements \Stringable {
         public string $value;
         // Further properties omitted for brevity.

         public function add(Number|string|int $num): Number {}
         // Further methods omitted for brevity.
     }

>> - I don't want to expand the scope of your RFC further than necessary, but for the rounding mode, I wonder if we should first add the RoundingMode enum that has been suggested during the discussion of the "Add 4 new rounding modes to round() function" RFC. It would make the type for the `Number::$roundMode` property much clearer than the current `int`. I expect the addition of such an enum to be a pretty simple RFC that would not need much discussion. I'd be happy to co-author it.
> 
> Oh, that's a good idea. Makes sense. I think it would be simpler to prepare an RFC separate from this RFC, so I'm going to create one right away. Once you have a certain amount of content, I would be happy if you could check it out and make corrections if necessary.
> 

Sure, just send me an email and I'm happy to look over it before you put 
it up for discussion.

>> - For `format()`, I'm not sure whether we should actually add the function. While we have number_format() for regular numbers with the same signature, it fails to account for the different language and cultures in the world. Specifically `number_format()` cannot correctly format numbers for en_IN (i.e. English in India). In India numbers are not separated every three digits, but rather the three right-most digits and then every two digits. Here's in example: 1,23,45,67,890. I believe formatting should be best left for ext/intl.
> 
> I'm not very familiar with ext/intl, but is there a way to format a string type number without converting it to a float?

It appears the underlying icu4c library supports formatting numeric 
strings, yes:

https://unicode-org.github.io/icu-docs/apidoc/dev/icu4c/classicu_1_1NumberFormat.html#a8ed2ca7b9a65bf08c4ef81bbf9680f0d

>> - I'm not sure if the priority for the rounding modes is sound. My gut feeling is that operations on numbers with different rounding modes should be disallowed or made explicit during the operation (much like the scale for a division), but I'm not an expert in designing numeric APIs, so I might be wrong here.
> 
> As mentioned in the RFC, the problem here is commutative calculations (add, sub, mul). This means that even if specify `$roundingMode` during these calculations, `$roundingMode` will not be used in these calculations. Calculations that use `$roundingMode` are divs, etc. If allow `$roundingMode` to be specified with add, intuitively it feels like the result of add will be rounded.
> 
> Also, it is not possible to specify `$roundMode` in calculations using operators.
> 
> However, if the user calculates two numbers with different rounding modes, and it is intentional rather than a mistake, I can't imagine what kind of result the user would want to get. Therefore, it may be better to make this an error.
> 
> Is it appropriate to throw a value error in that case?

I think making this an error would be appropriate. Generally speaking: 
Removing errors later is always possible if we find out that an 
operation is safe and well-defined. Adding an error if we find out that 
an operation is unsafe will result in breaking changes, thus we should 
get it right on the first try.

Best regards
Tim Düsterhus