Hi internals,
I want to start the discussion on the PHP RFC: Support object type in BCMath.
https://wiki.php.net/rfc/support_object_type_in_bcmath
Regards.
Saki
On 24/03/2024 13:13, Saki Takamachi wrote:
I want to start the discussion on the PHP RFC: Support object type in BCMath.
I suggest renaming `setScale` to `withScale`. Although the docs will make clear that the object is immutable, `set` is associated with mutation and might be confusing. `with` is not as well known as a prefix but is associated with immutable objects. Also as with the value, any reason not to make the scale a pubic readonly property?
On 24/03/2024 13:13, Saki Takamachi wrote:
I want to start the discussion on the PHP RFC: Support object type in BCMath.
I work on OLTP application using BCMath for money, and I would like to refactor to value objects (although we could also convert to using ints), so this is very relevant to me. Liking the RFC a lot. Is there any reason not to give the BcNum class a public readonly string `value` property? Would just save a few characters of typing to use value instead of getValue().
On 24/03/2024 13:13, Saki Takamachi wrote:
I want to start the discussion on the PHP RFC: Support object type in BCMath.
One more suggestion - might it be worth adding a `format` function to the new BcNum class? This would be similar to the existing number_format function, but would avoid the need to lose precision by converting to float first.
Hi Barney, thanks for the points and suggestions!
Is there any reason not to give the BcNum class a public readonly string `value` property? Would just save a few characters of typing to use value instead of getValue().
Also as with the value, any reason not to make the scale a pubic readonly property?
I had completely forgotten about the existence of read-only properties. That makes sense.
I suggest renaming `setScale` to `withScale`. Although the docs will make clear that the object is immutable, `set` is associated with mutation and might be confusing. `with` is not as well known as a prefix but is associated with immutable objects.
Indeed, I felt uncomfortable using "set”. I didn't know that "with" was related to immutable.
**I immediately reflected the above two points in my RFC** 
One more suggestion - might it be worth adding a `format` function to the new BcNum class? This would be similar to the existing number_format function, but would avoid the need to lose precision by converting to float first.
I came up with the following code, is it close to what you intended?
$num = BcNum::fromNumberFormat(1.2345, 5);
$num->value; // 1.23450
Regards.
Saki
On 2024-03-26 11:35, Saki Takamachi wrote:
**I immediately reflected the above two points in my RFC**
Thanks, looks good.
One more suggestion - might it be worth adding a `format` function to the new BcNum class? This would be similar to the existing number_format function, but would avoid the need to lose precision by converting to float first.
I came up with the following code, is it close to what you intended?
$num = BcNum::fromNumberFormat(1.2345, 5); $num->value; // 1.23450
No, that's not quite what I meant - I meant more like the opposite:
$bcNum = new BcNum('1234567890123456789.23456789');
echo $bcNum->format(8, '.', ',') // 1,234,567,890,123,456,789.23456789
Maybe also worth providing a way to specify that all decimals should be printed, instead of just a fixed number of decimals.
On 24.03.2024 14:13, Saki Takamachi wrote:
Hi internals,
I want to start the discussion on the PHP RFC: Support object type in BCMath.
Was BCMath\Number considered instead of BcNum?
ps. there's '2,111' in one place, but should be '2.111', I guess.
--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
On 26.03.2024 14:35, Saki Takamachi wrote:
Hi Aleksander,
Was BCMath\Number considered instead of BcNum?
Yes, that was one of the candidates. However, as far as I know, there are no examples of PHP internal classes having namespaces.
Also, if use a namespace, the code will be written as `new Number()`, which is likely to conflict with existing code. In fact, if take a look at GitHub Code Search, you'll find 3.2k results.
Code search results · GitHubThis won't result in a BC Break, but it can be a bit difficult to use.
After reading PHP: rfc:namespaces_in_bundled_extensions again I see it is a perfect case to apply it. While it's not a must, I think we should go with BCMath/Number.
--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
Hi Barney,
No, that's not quite what I meant - I meant more like the opposite:
$bcNum = new BcNum('1234567890123456789.23456789'); echo $bcNum->format(8, '.', ',') // 1,234,567,890,123,456,789.23456789Maybe also worth providing a way to specify that all decimals should be printed, instead of just a fixed number of decimals.
Ah I see! It sounds exactly like `number_format`. Sure, this might make sense. To me, this seems worth supporting.
BTW,
$bcNum = new BcNum('1234567890123456789.23456789’);
When I saw this, I thought that the behavior when $scale is omitted is that in addition to the option of "using the global setting value", there is also the option of "counting the length of `$num` and storing all `$num`”. In other words, it does not use any global settings.
Which of these do you think is better?
Regards.
Saki
Hi Aleksander,
Was BCMath\Number considered instead of BcNum?
Yes, that was one of the candidates. However, as far as I know, there are no examples of PHP internal classes having namespaces.
Also, if use a namespace, the code will be written as `new Number()`, which is likely to conflict with existing code. In fact, if take a look at GitHub Code Search, you'll find 3.2k results.
https://github.com/search?type=code&auto_enroll=true&q="new+Number("+language%3APHP+
This won't result in a BC Break, but it can be a bit difficult to use.
ps. there's '2,111' in one place, but should be '2.111', I guess.
Oops, thank you. You have good eyes.
Regards.
Saki
Hi Aleksander,
After reading PHP: rfc:namespaces_in_bundled_extensions again I see it is a perfect case to apply it. While it's not a must, I think we should go with BCMath/Number.
Thank you, I have read it now. Certainly in this case it makes sense to use "BcMath" as the namespace ("BcMath" is appropriate instead of "BCMath" as it must follow PHP naming conventions).
What concerns me is that the symbol "Number" is difficult to understand in the code. So, how about "BcMath\BcNum”?
Regards.
Saki
On Sun, 24 Mar 2024, Saki Takamachi wrote:
Hi internals,
I want to start the discussion on the PHP RFC: Support object type in BCMath.
I have some comments:
- You've picked as class name "BcNum". Following
our naming guidelines, that probably should be \BCMath\Num (or
\BC\Num, but that is less descriptive):
policies/coding-standards-and-naming.rst at main · php/policies · GitHub
The reason it *should* have "BC" is that it comes from "Basic
Calculator" (PHP: BC Math - Manual)
- Should ->value rather be ->toString() ? ->value alone doesn't really
say much. I'm on the fence here though, as there is already
(internally) a ->__toString() method to make the (string) cast work.
- Would it make sense to have "floor" and "ceil" to also have a scale,
or precision? Or would developers instead have to use "round" in that
case?
- Which rounding modes are supported with "round", the same ones as the
normal round() function?
- In this example, what would $result->scale show? (Perhaps add that to
the example?):
<?php
$num = new BcNum('1.23', 2);
$result = $num + '1.23456';
$result->value; // '2.46456'
$result->scale; // ??
- Exceptions
The RFC does not mention which exceptions can be thrown. Is it just
the one? It might be beneficial to *do* have a new exception
hierarchy.
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: Xdebug: Support
mastodon: @derickr@phpc.social @xdebug@phpc.social
On Tue, Mar 26, 2024, at 12:50 PM, Saki Takamachi wrote:
Hi Barney,
No, that's not quite what I meant - I meant more like the opposite:
$bcNum = new BcNum('1234567890123456789.23456789'); echo $bcNum->format(8, '.', ',') // 1,234,567,890,123,456,789.23456789Maybe also worth providing a way to specify that all decimals should be printed, instead of just a fixed number of decimals.
Ah I see! It sounds exactly like `number_format`. Sure, this might make
sense. To me, this seems worth supporting.BTW,
$bcNum = new BcNum('1234567890123456789.23456789’);When I saw this, I thought that the behavior when $scale is omitted is
that in addition to the option of "using the global setting value",
there is also the option of "counting the length of `$num` and storing
all `$num`”. In other words, it does not use any global settings.Which of these do you think is better?
Global mode settings are an anti-pattern in most cases. Please avoid those whenever possible, as they lead to unpredictable behavior.
--Larry Garfield
Hi Larry,
Global mode settings are an anti-pattern in most cases. Please avoid those whenever possible, as they lead to unpredictable behavior.
Yes, that's right.
BCMath has an existing global setting, so I was wondering if it was something I could ignore. But that means there's no reason to use global settings other than "they exist in existing implementations"…
Okay, regarding the existing global setting, I decided not to support it with BcNum.
Regards.
Saki
Hi,
I wrote in my RFC that it does not support global settings.
Regards.
Saki
On 24.03.2024 14:13, Saki Takamachi wrote:
Hi internals,
I want to start the discussion on the PHP RFC: Support object type in BCMath.
Here's another question.
1. Since we have withScale(), do we need to inherit the $scale argument from the functional API? Can't we derive it from the object the method is being invoked on?
So, instead, e.g.
public function add(BcNum|string|int $num, ?int $scale = null): BcNum {}
public function sqrt(?int $scale = null): BcNum {}
I'd suggest:
public function add(BcNum|string|int $num): BcNum {}
public function sqrt(): BcNum {}
but I have no clue about BCMath.
--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com
On 2024-03-24 13:13, Saki Takamachi wrote:
I want to start the discussion on the PHP RFC: Support object type in BCMath.
Do we also need `toFloat` and `toInt` functions? Seems like using explicit functions will be safer than casting.
For toInt I'd expect an exception if the value is outside the range of possible ints. For toFloat it might be nice to have a flag
argument to give the developer the choice of having it throw if the value is outside the range of floats or return INF or -INF,
or possibly the user should just check for infinite values themselves.
Hi Derick,
- You've picked as class name "BcNum". Following
our naming guidelines, that probably should be \BCMath\Num (or
\BC\Num, but that is less descriptive):
policies/coding-standards-and-naming.rst at main · php/policies · GitHubThe reason it *should* have "BC" is that it comes from "Basic
Calculator" (PHP: BC Math - Manual)
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
say much. I'm on the fence here though, as there is already
(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,
or precision? Or would developers instead have to use "round" in that
case?
- Which rounding modes are supported with "round", the same ones as the
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
the example?):<?php
$num = new BcNum('1.23', 2);
$result = $num + '1.23456';
$result->value; // '2.46456'
$result->scale; // ??
In this case, `$result->scale` will be `'5'`. I added this to the RFC.
- Exceptions
The RFC does not mention which exceptions can be thrown. Is it just
the one? It might be beneficial to *do* have a new exception
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
Hi Aleksander,
Here's another question.
1. Since we have withScale(), do we need to inherit the $scale argument from the functional API? Can't we derive it from the object the method is being invoked on?
So, instead, e.g.
public function add(BcNum|string|int $num, ?int $scale = null): BcNum {}
public function sqrt(?int $scale = null): BcNum {}I'd suggest:
public function add(BcNum|string|int $num): BcNum {}
public function sqrt(): BcNum {}but I have no clue about BCMath.
Yeah, you're right. By using `withScale` before calculating, we can obtain results of arbitrary precision without using Scale during calculation.
The code in that case would look like this:
$num = new Number('1.23');
$num2 = new Number('4.56');
$numRescaled = $num->withScale(4);
$result = $numRescaled->add($num2);
$result->value; // '5.7900'
$result->scale; // 4
On the other hand, if allow the calculation method to specify a scale, we can write it like this:
$num = new Number('1.23');
$num2 = new Number('4.56');
$result = $num->add($num2, 4);
$result->value; // '5.7900'
$result->scale; // 4
It's just one less line, but that's the only reason to support the `$scale` argument.
However, for calculations other than mul and div, the calculation results will always fit within the scale.
$num = new Number('1.23'); // scale 2
$num2 = new Number('1'); // scale 0
$num->add($num2); // '2.23', scale 2 is enough
$num->sub($num2); // '0.23', scale 2 is enough
$num->mod($num2); // '0.23', scale 2 is enough
On the other hand, for mul, div, and pow, the original `$num` scale may not be enough.
$num = new Number('1.23'); // scale 2
$num2 = new Number('1.1'); // scale 1
$num->mul($num2); // '1.353' be '1.35', scale 2 is not enough
$num = new Number('1.23'); // scale 2
$num2 = new Number('4'); // scale 0
$num->div($num2); // '0.3075' be '0.30', scale 2 is not enough
$num->pow($num2); // '2.28886641', be '2.28' scale 2 is not enough
For mul and pow, can calculate the required scale. However, for div, the calculation may never end, such as `0.33333....`, so it is not possible to calculate the scale to fit the result completely.
In cases like this, we thought that some users would want to easily specify the scale, so we created an easy-to-use signature.
However, it may not be necessary to specify scale for all calculation functions. Is it reasonable to specify only mul, div, pow, or only div?
Regards.
Saki
Hi Derick,
I made one mistake.
In this case, `$result->scale` will be `'5'`. I added this to the RFC.
It's `5`, not `'5’`.
Regards.
Saki