Hi
On 4/6/24 17:07, Saki Takamachi wrote:
To you all:
The discussions thus far have been reflected in the RFC. I would be very happy if you
could check it out. (Thanks Tim, the <PHP> tag is very easy to read.)
https://wiki.php.net/rfc/support_object_type_in_bcmath
Thank you for the updates. Some remarks:
- The namespace is inconsistently referred to as "BCMath" and "BcMath",
please check the entire RFC to unify the casing.
- The full “stub” should probably include an explicit "implements
Stringable" for clarity.
- 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.
- The property should probably be named "$roundingMode" instead of
"$roundMode".
- I'm not sure I like the differing default rounding modes for implicit
rounding (i.e. the $roundMode property) and explicit rounding (i.e. the
round() method). That sounds like it would cause unnecessary confusion.
- In the stub there's a typo: "puclic", should be "public".
- 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'd probably not add the 'with()' method, because it would be
redundant with https://wiki.php.net/rfc/clone_with and it can already be
specified by a combination of withMaxExpansionScale + withRoundMode.
- 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.
- For the RFC Impact on SAPIs, it should simply be "None", because SAPIs
do not need to do anything special.
- For the Backwards Incompatible Changes, it should list that the
BcMath\Number class will no longer be available to userland.
Best regards
Tim Düsterhus
