[RFC] [Discussion] Support object type in BCMath

1 year ago by Barney Laurance — view source

unread

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?

1 year ago by Barney Laurance — view source

unread

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().

1 year ago by Barney Laurance — view source

unread

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

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.

1 year ago by Saki Takamachi — view source

unread

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 :D

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

1 year ago by Barney Laurance — view source

unread

I immediately reflected the above two points in my RFC :D

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.

1 year ago by Saki Takamachi — view source

unread

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.23456789
Maybe 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

1 year ago by Larry Garfield — view source

unread

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.23456789
Maybe 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

1 year ago by Saki Takamachi — view source

unread

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

1 year ago by Saki Takamachi — view source

unread

Hi,

I wrote in my RFC that it does not support global settings.

Regards.

Saki

1 year ago by Aleksander Machniak — view source

unread

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

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

1 year ago by Saki Takamachi — view source

unread

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=%22new+Number%28%22+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

1 year ago by Aleksander Machniak — view source

unread

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=%22new+Number%28%22+language%3APHP+

This won't result in a BC Break, but it can be a bit difficult to use.

After reading https://wiki.php.net/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

1 year ago by Saki Takamachi — view source

unread

Hi Aleksander,

After reading https://wiki.php.net/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

1 year ago by Derick Rethans — view source

unread

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

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):
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#namespaces-in-extensions

The reason it should have "BC" is that it comes from "Basic
Calculator" (https://www.php.net/manual/en/book.bc.php#118203)
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: https://xdebug.org/support

mastodon: @derickr@phpc.social @xdebug@phpc.social

1 year ago by Saki Takamachi — view source

unread

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):
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#namespaces-in-extensions

The reason it should have "BC" is that it comes from "Basic
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
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

1 year ago by Saki Takamachi — view source

unread

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

1 year ago by Aleksander Machniak — view source

unread

$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.

I'm not sure I like this. Maybe we should be more strict here and treat
the $scale in constructor (and later withScale()) as the actual scale
for all operations.

So, in the case above I'd expect ->scale to be 2, and ->value to be
'2.46'. If I wanted $num to have a scale that may change, I'd not define
it in the first place. Does that make sense?

ps. that also means withScale(null) should be possible.

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

1 year ago by Saki Takamachi — view source

unread

Hi Aleksander,

If you write it as:

$result = $num->withScale(4)->add($num2);

it's not an extra line anymore. I also think that withScale() use will be rare, as we have the scale in constructor.

I think the intention is more clear here, and I think it applies to all cases you mentioned, including div or pow. If you know you need to change the scale just add ->withScale(X) before.

Ah, that's right. I wonder why I forgot about method chaining. Update RFC. If we miss something and $scale is needed, I can always revert the RFC.

I'm not sure I like this. Maybe we should be more strict here and treat the $scale in constructor (and later withScale()) as the actual scale for all operations.

So, in the case above I'd expect ->scale to be 2, and ->value to be '2.46'. If I wanted $num to have a scale that may change, I'd not define it in the first place. Does that make sense?

ps. that also means withScale(null) should be possible.

I see, that may indeed make sense. Now here we have three choices about it. In fact, if want to behave like the current RFC, we can do it by calculating as $num + new Number($str).
Yeah, I'll fix the RFC for this.

Regards.

Saki

1 year ago by Jordan LeDoux — view source

unread

$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.

I'm not sure I like this. Maybe we should be more strict here and treat
the $scale in constructor (and later withScale()) as the actual scale
for all operations.

For addition, it absolutely should expand scale like this, unless the
constructor also defines a default rounding type that is used in that
situation. All numbers, while arbitrary, will be finite, so addition will
always be exact and known based on inputs prior to calculation.

Treating scale like this isn't more strict, it's confusing. For instance:

$numA = new Number('1.23', 2);
$numB = new Number('1.23456', 5);

$expandedScale1 = $numA + $numB; // 2.46456
$expandedScale2 = $numB + $numA; // 2.46456

$strictScale1 = $numA + $numB; // 2.46 assuming truncation
$strictScale2 = $numB + $numA; // 2.46456

I ran into this same issue with operand ordering when I was writing my
operator overload RFC.

There are ways you could do the overload implementation that would get
around this for object + object operations, but it's also mathematically
unsound and probably unexpected for anyone who is going to the trouble of
using an arbitrary precision library.

Addition and subtraction should automatically use the largest scale from
all operands. Division and multiplication should require a specified scale.

Because of this, I'm not entirely sure that specifying a scale in the
constructor is actually a good thing. It is incredibly easy to create
situations, unless the implementation in C is VERY careful, where the
operand positions matter beyond the simple calculation. Multiplication is
commutative, but division is not. This would almost certainly lead to some
very difficult to track down bugs.

Putting scale in the constructor is similar to some of the examples of
"possible misuse cases of operator overloading" that I had to go over when
I was making my RFC. We definitely want to avoid that if possible for the
first number/math object that has operator overloads.

Jordan

1 year ago by Saki Takamachi — view source

unread

Hi Jordan,

For addition, it absolutely should expand scale like this, unless the constructor also defines a default rounding type that is used in that situation. All numbers, while arbitrary, will be finite, so addition will always be exact and known based on inputs prior to calculation.

Treating scale like this isn't more strict, it's confusing. For instance:
$numA = new Number('1.23', 2);
$numB = new Number('1.23456', 5);

$expandedScale1 = $numA + $numB; // 2.46456
$expandedScale2 = $numB + $numA; // 2.46456

$strictScale1 = $numA + $numB; // 2.46 assuming truncation
$strictScale2 = $numB + $numA; // 2.46456
I ran into this same issue with operand ordering when I was writing my operator overload RFC.

There are ways you could do the overload implementation that would get around this for object + object operations, but it's also mathematically unsound and probably unexpected for anyone who is going to the trouble of using an arbitrary precision library.

Addition and subtraction should automatically use the largest scale from all operands. Division and multiplication should require a specified scale.

Because of this, I'm not entirely sure that specifying a scale in the constructor is actually a good thing. It is incredibly easy to create situations, unless the implementation in C is VERY careful, where the operand positions matter beyond the simple calculation. Multiplication is commutative, but division is not. This would almost certainly lead to some very difficult to track down bugs.

Putting scale in the constructor is similar to some of the examples of "possible misuse cases of operator overloading" that I had to go over when I was making my RFC. We definitely want to avoid that if possible for the first number/math object that has operator overloads.

Your opinion may be reasonable given the original BCMath calculation order. That is, do you intend code like this?

Signature:

// public function __construct(string|int $number)
// public function getNumber(?int $scale = null): string

Add:

// public function add(Number|string|int $number): string

$num = new Number('1.23456');
$num2 = new Number('1.23');

$add = $num + $num2;
$add->getNumber(); // '2.46456'
$add->getNumber(1); // ‘2.4'

$add = $num->add($num2);
$add->getNumber(); // '2.46456'
$add->getNumber(1); // '2.4'

Div:

// public function div(Number|string|int $number, int $scaleExpansionLimit = 10): string


// case 1
$num = new Number('0.0001');
$num2 = new Number('3');

$div = $num / $num2; // scale expansion limit is always 10
$div->getNumber(); // '0.0000333333333'

$div = $num->div($num2, 20);
$div->getNumber(); // '0.00003333333333333333333'
$div->getNumber(7); // ‘0.0000333'


// case 2
$num = new Number('1.111111');
$num2 = new Number('3');

$div = $num->div($num2, 3);
$div->getNumber(); // '0.370'
$div->getNumber(7); // ‘0.3700000'

Since the scale can be inferred for everything other than div, a special argument is given only for div.

Regards.

Saki

1 year ago by Saki Takamachi — view source

unread

Another div case:


$num = new Number('0.000000000000000001'); // scale 18
$num2 = new Number('3');

$div = $num->div($num2);
$div->getNumber(); // '0.000000000000000000' scale 18

$div = $num->div($num2, 20);
$div->getNumber(); // '0.00000000000000000033' scale 20

Regards.

Saki

11 months ago by Jordan LeDoux — view source

unread

Hi Jordan,

Your opinion may be reasonable given the original BCMath calculation
order. That is, do you intend code like this?

Signature:

// public function __construct(string|int $number)
// public function getNumber(?int $scale = null): string

Add:

// public function add(Number|string|int $number): string

$num = new Number('1.23456');
$num2 = new Number('1.23');

$add = $num + $num2;
$add->getNumber(); // '2.46456'
$add->getNumber(1); // ‘2.4'

$add = $num->add($num2);
$add->getNumber(); // '2.46456'
$add->getNumber(1); // '2.4'

Div:

// public function div(Number|string|int $number, int $scaleExpansionLimit
= 10): string


// case 1
$num = new Number('0.0001');
$num2 = new Number('3');

$div = $num / $num2; // scale expansion limit is always 10
$div->getNumber(); // '0.0000333333333'

$div = $num->div($num2, 20);
$div->getNumber(); // '0.00003333333333333333333'
$div->getNumber(7); // ‘0.0000333'


// case 2
$num = new Number('1.111111');
$num2 = new Number('3');

$div = $num->div($num2, 3);
$div->getNumber(); // '0.370'
$div->getNumber(7); // ‘0.3700000'

Since the scale can be inferred for everything other than div, a special
argument is given only for div.

Regards.

Saki

Something like the signature for getNumber() in this example would be a
decent solution. Operations which have ambiguous scale (of which truly only
div is in the BCMath library) should require scale in the method that
calls the calculation, however for consistency I can certainly see the
argument for requiring it for all calculation methods. The issue is how you
want to handle that for operator overloads, since you cannot provide
arguments in that situation.

Probably the most sensible way (and I think the way I handled it as well in
my library) is to look at both the left and right operand, grab the
calculated scale of the input for both (or the set scale if the scale has
been manually set), and then calculate with a higher scale. If internally
it produces a rounded result, the calculation should be done at
$desireScale + 2 to avoid compound rounding errors from the BCMath
library and then the implementation. If the result is truncated, the
calculation should be done at $desiredScale + 1 to avoid calculating
unnecessary digits.

So we have multiple usage scenarios and the behavior needs to remain
consistent no matter which usage occurs, and what order the items are
called in, so long as the resulting calculation is the same.

Method Call
$bcNum = new Number('1.0394567'); // Input scale is implicitly 7
$bcNum->div('1.2534', 3); // Resulting scale is 3
$bcNum->div('1.2534'); // Implicit scale of denominator is 4, Implicit
scale of numerator is 7, calculate with scale of 8 then truncate

Operators
$bcNum = new Number('1.0394567'); // Input scale is implicitly 7
$bcNum / '1.2534'; // Implicit scale of denominator is 4, Implicit scale of
numerator is 7, calculate with scale of 8 then truncate

This allows you to perhaps keep an input scale in the constructor and also
maintain consistency across various calculations. But whatever the behavior
is, it should be mathematically sound, consistent across different syntax
for the same calculation, and never reducing scale UNLESS it is told to do
so in the calculation step OR during the value retrieval.

Jordan

11 months ago by Lynn — view source

unread

On Tue, Apr 2, 2024 at 11:17 AM Jordan LeDoux jordan.ledoux@gmail.com
wrote:

Hi Jordan,

Your opinion may be reasonable given the original BCMath calculation
order. That is, do you intend code like this?

Signature:
// public function __construct(string|int $number)
// public function getNumber(?int $scale = null): string
Add:
// public function add(Number|string|int $number): string

$num = new Number('1.23456');
$num2 = new Number('1.23');

$add = $num + $num2;
$add->getNumber(); // '2.46456'
$add->getNumber(1); // ‘2.4'

$add = $num->add($num2);
$add->getNumber(); // '2.46456'
$add->getNumber(1); // '2.4'
Div:
// public function div(Number|string|int $number, int
$scaleExpansionLimit = 10): string


// case 1
$num = new Number('0.0001');
$num2 = new Number('3');

$div = $num / $num2; // scale expansion limit is always 10
$div->getNumber(); // '0.0000333333333'

$div = $num->div($num2, 20);
$div->getNumber(); // '0.00003333333333333333333'
$div->getNumber(7); // ‘0.0000333'


// case 2
$num = new Number('1.111111');
$num2 = new Number('3');

$div = $num->div($num2, 3);
$div->getNumber(); // '0.370'
$div->getNumber(7); // ‘0.3700000'
Since the scale can be inferred for everything other than div, a special
argument is given only for div.

Regards.

Saki
Something like the signature for getNumber() in this example would be a
decent solution. Operations which have ambiguous scale (of which truly only
div is in the BCMath library) should require scale in the method that
calls the calculation, however for consistency I can certainly see the
argument for requiring it for all calculation methods. The issue is how you
want to handle that for operator overloads, since you cannot provide
arguments in that situation.

Probably the most sensible way (and I think the way I handled it as well
in my library) is to look at both the left and right operand, grab the
calculated scale of the input for both (or the set scale if the scale has
been manually set), and then calculate with a higher scale. If internally
it produces a rounded result, the calculation should be done at
$desireScale + 2 to avoid compound rounding errors from the BCMath
library and then the implementation. If the result is truncated, the
calculation should be done at $desiredScale + 1 to avoid calculating
unnecessary digits.

So we have multiple usage scenarios and the behavior needs to remain
consistent no matter which usage occurs, and what order the items are
called in, so long as the resulting calculation is the same.

Method Call
$bcNum = new Number('1.0394567'); // Input scale is implicitly 7
$bcNum->div('1.2534', 3); // Resulting scale is 3
$bcNum->div('1.2534'); // Implicit scale of denominator is 4, Implicit
scale of numerator is 7, calculate with scale of 8 then truncate

Operators
$bcNum = new Number('1.0394567'); // Input scale is implicitly 7
$bcNum / '1.2534'; // Implicit scale of denominator is 4, Implicit scale
of numerator is 7, calculate with scale of 8 then truncate

This allows you to perhaps keep an input scale in the constructor and also
maintain consistency across various calculations. But whatever the behavior
is, it should be mathematically sound, consistent across different syntax
for the same calculation, and never reducing scale UNLESS it is told to do
so in the calculation step OR during the value retrieval.

Jordan

I'm inexperienced when it comes to maths and the precision here, but I do
have some experience when it comes to what the business I work for wants.
I've implemented BCMath in a couple of places where this kind of precision
is necessary, and I found that whenever I do divisions I prefer having at
least 2 extra digits. Would it make sense to internally always just store a
more accurate number? For things like
additions/multiplications/subtractions it could always use the highest
precision, and then for divisions add like +3~6 or something. Whenever you
have numbers that have a fraction like 10.5001 it makes sense to set it
to 4, but when you have 10 it suddenly becomes 0 when implicitly setting
it.

For the following examples assume each number is a BcNum:
When doing something like 10 * 10.0000 * 10.000000000 I want the end
result to have a precision of at least 9 so I don't lose information. When
I do ((10 / 3) * 100) * 2 I don't want it to implicitly become 0, because
the precision here is important to me. I don't think using infinite
precision here is a reasonable approach either. I'm not sure what the
correct answer is, perhaps it's just "always manually set the precision"?

11 months ago by Saki Takamachi — view source

unread

Hi Jordan, Lynn,

Something like the signature for getNumber() in this example would be a decent solution. Operations which have ambiguous scale (of which truly only div is in the BCMath library) should require scale in the method that calls the calculation, however for consistency I can certainly see the argument for requiring it for all calculation methods. The issue is how you want to handle that for operator overloads, since you cannot provide arguments in that situation.

Probably the most sensible way (and I think the way I handled it as well in my library) is to look at both the left and right operand, grab the calculated scale of the input for both (or the set scale if the scale has been manually set), and then calculate with a higher scale. If internally it produces a rounded result, the calculation should be done at $desireScale + 2 to avoid compound rounding errors from the BCMath library and then the implementation. If the result is truncated, the calculation should be done at $desiredScale + 1 to avoid calculating unnecessary digits.

So we have multiple usage scenarios and the behavior needs to remain consistent no matter which usage occurs, and what order the items are called in, so long as the resulting calculation is the same.

Method Call
$bcNum = new Number('1.0394567'); // Input scale is implicitly 7
$bcNum->div('1.2534', 3); // Resulting scale is 3
$bcNum->div('1.2534'); // Implicit scale of denominator is 4, Implicit scale of numerator is 7, calculate with scale of 8 then truncate

Operators
$bcNum = new Number('1.0394567'); // Input scale is implicitly 7
$bcNum / '1.2534'; // Implicit scale of denominator is 4, Implicit scale of numerator is 7, calculate with scale of 8 then truncate

This allows you to perhaps keep an input scale in the constructor and also maintain consistency across various calculations. But whatever the behavior is, it should be mathematically sound, consistent across different syntax for the same calculation, and never reducing scale UNLESS it is told to do so in the calculation step OR during the value retrieval.

I'm inexperienced when it comes to maths and the precision here, but I do have some experience when it comes to what the business I work for wants. I've implemented BCMath in a couple of places where this kind of precision is necessary, and I found that whenever I do divisions I prefer having at least 2 extra digits. Would it make sense to internally always just store a more accurate number? For things like additions/multiplications/subtractions it could always use the highest precision, and then for divisions add like +3~6 or something. Whenever you have numbers that have a fraction like 10.5001 it makes sense to set it to 4, but when you have 10 it suddenly becomes 0 when implicitly setting it.

For the following examples assume each number is a BcNum:
When doing something like 10 * 10.0000 * 10.000000000 I want the end result to have a precision of at least 9 so I don't lose information. When I do ((10 / 3) * 100) * 2 I don't want it to implicitly become 0, because the precision here is important to me. I don't think using infinite precision here is a reasonable approach either. I'm not sure what the correct answer is, perhaps it's just "always manually set the precision"?

Thanks for the important perspective feedback.

One thing I overlooked: if the exponent of pow is negative, the scale of the result becomes unpredictable, just like with div.

e.g.

3 ** -1
= 0.333333.....

Also, an idea occurred to me while reading your comments.

The current assumption is that a Number always holds a single value. How if we made it so that it held two values? They are the numerator and the denominator.

This means that when we do division, no division is done internally, but we actually multiply the denominator. At the very end of the process, when converting to string, any reserved division is performed according to the specified scale.

If we have the option of not specifying a scale when converting to a string, it may be preferable to convert based on an implicit scale.

Regards.

Saki

11 months ago by Barney Laurance — view source

unread

Also, an idea occurred to me while reading your comments.

The current assumption is that a Number always holds a single value.
How if we made it so that it held two values? They are the numerator
and the denominator.

Then we'd have a rational number, instead of an arbitrary precision
decimal. I think that's a sufficiently different data type that it
should be a different class (if required), and probably a separate RFC,
and for now it's better to stay closer to the existing BCMath API.

Developers should be prepared to accept that an arbitrary precision
decimal can't represent 1/3 exactly, just like a binary float can't
represent 1/10 exactly.

11 months ago by Jordan LeDoux — view source

unread

I'm inexperienced when it comes to maths and the precision here, but I do
have some experience when it comes to what the business I work for wants.
I've implemented BCMath in a couple of places where this kind of precision
is necessary, and I found that whenever I do divisions I prefer having at
least 2 extra digits. Would it make sense to internally always just store a
more accurate number? For things like
additions/multiplications/subtractions it could always use the highest
precision, and then for divisions add like +3~6 or something. Whenever you
have numbers that have a fraction like 10.5001 it makes sense to set it
to 4, but when you have 10 it suddenly becomes 0 when implicitly setting
it.

For the following examples assume each number is a BcNum:
When doing something like 10 * 10.0000 * 10.000000000 I want the end
result to have a precision of at least 9 so I don't lose information. When
I do ((10 / 3) * 100) * 2 I don't want it to implicitly become 0, because
the precision here is important to me. I don't think using infinite
precision here is a reasonable approach either. I'm not sure what the
correct answer is, perhaps it's just "always manually set the precision"?

In my library, if the scale is unspecified, I actually set the scale to 10
OR the length of the input string, including integer decimals, whichever is
larger. Since I was designing my own library I could do things like that as
convention, and a scale of 10 is extremely fast, even with the horrifically
slow BCMath library, but covers most use cases (the overwhelmingly common
of which is exact calculation of money).

My library handles scale using the following design. It's not necessarily
correct here, as I was designing a PHP library instead of something for
core, AND my library does not have to deal with operator overloads so I'm
always working with method signatures instead, AND it's possible that my
class/method design is inferior to other alternatives, however it went:

Each number constructor allowed for an optional input scale.
The input number was converted into the proper formatting from allowed
input types, and then the implicit scale is set to the total number of
digits.
If the input scale was provided, the determined scale is set to that
value.
Otherwise, the determined scale at construction is set to 10 or the
implicit scale of "number of digits", whichever is larger.
The class contained the roundToScale method, which allowed you to
provide the desired scale and the rounding method, and then would set the
determined scale to that value after rounding. It contained the round
method with the same parameters to allow rounding to a specific scale
without also setting the internal determined scale at the same time.
The class contained the setScale method which set the value of the
internal determined scale value to an int without mutating the value at all.
All mathematical operation methods which depended on scale, (such as div
or pow), allowed an optional input scale that would be used for calculation
if present. If it was not present, the internal calculations were done by
taking the higher of the determined scale between the two operands, and
then adding 2, and then the result was done by rounding using the default
method of ROUND_HALF_EVEN if no rounding method was provided.

Again, though I have spent a lot of design time on this issue for the math
library I developed, my library did not have to deal with the RFC process
for PHP or maintain consistency with the conventions of PHP core, only with
the conventions it set for itself. However, I can provide a link to the
library for reference on the issue if that would be helpful for people that
are contributing to the design aspects of this RFC.

The current assumption is that a Number always holds a single value. How
if we made it so that it held two values? They are the numerator and the
denominator.

Again, my experience on the issue is with the development of my own library
on the issue, however in my case I fully separated that kind of object into
its own class Fraction, and gave the kinds of operations we've been
discussing to the class Decimal. Storing numerators and denominators for
as long as possible involves a completely different set of math. For
instance, you need an algorithm to determine the Greatest Common Factor and
the Least Common Multiple in such a class, because there are a lot of
places where you would need to find the smallest common denominator or
simplify the fraction.

Abstracting between the Fraction and Decimal so that they worked with
each other honestly introduced the most complex and inscrutable code in my
entire library, so unless fractions are themselves also a design goal of
this RFC, I would recommend against it.

Jordan

11 months ago by Jordan LeDoux — view source

unread

On Tue, Apr 2, 2024 at 10:24 AM Jordan LeDoux jordan.ledoux@gmail.com
wrote:

I'm inexperienced when it comes to maths and the precision here, but I do
have some experience when it comes to what the business I work for wants.
I've implemented BCMath in a couple of places where this kind of precision
is necessary, and I found that whenever I do divisions I prefer having at
least 2 extra digits. Would it make sense to internally always just store a
more accurate number? For things like
additions/multiplications/subtractions it could always use the highest
precision, and then for divisions add like +3~6 or something. Whenever you
have numbers that have a fraction like 10.5001 it makes sense to set it
to 4, but when you have 10 it suddenly becomes 0 when implicitly setting
it.

For the following examples assume each number is a BcNum:
When doing something like 10 * 10.0000 * 10.000000000 I want the end
result to have a precision of at least 9 so I don't lose information. When
I do ((10 / 3) * 100) * 2 I don't want it to implicitly become 0, because
the precision here is important to me. I don't think using infinite
precision here is a reasonable approach either. I'm not sure what the
correct answer is, perhaps it's just "always manually set the precision"?

In my library, if the scale is unspecified, I actually set the scale to 10
OR the length of the input string, including integer decimals, whichever is
larger. Since I was designing my own library I could do things like that as
convention, and a scale of 10 is extremely fast, even with the horrifically
slow BCMath library, but covers most use cases (the overwhelmingly common
of which is exact calculation of money).

My library handles scale using the following design. It's not necessarily
correct here, as I was designing a PHP library instead of something for
core, AND my library does not have to deal with operator overloads so I'm
always working with method signatures instead, AND it's possible that my
class/method design is inferior to other alternatives, however it went:

Each number constructor allowed for an optional input scale.

The input number was converted into the proper formatting from allowed
input types, and then the implicit scale is set to the total number of
digits.

If the input scale was provided, the determined scale is set to that
value.

Otherwise, the determined scale at construction is set to 10 or the
implicit scale of "number of digits", whichever is larger.

The class contained the roundToScale method, which allowed you to
provide the desired scale and the rounding method, and then would set the
determined scale to that value after rounding. It contained the round
method with the same parameters to allow rounding to a specific scale
without also setting the internal determined scale at the same time.

The class contained the setScale method which set the value of the
internal determined scale value to an int without mutating the value at all.

All mathematical operation methods which depended on scale, (such as
div or pow), allowed an optional input scale that would be used for
calculation if present. If it was not present, the internal calculations
were done by taking the higher of the determined scale between the two
operands, and then adding 2, and then the result was done by rounding using
the default method of ROUND_HALF_EVEN if no rounding method was provided.

Again, though I have spent a lot of design time on this issue for the math
library I developed, my library did not have to deal with the RFC process
for PHP or maintain consistency with the conventions of PHP core, only with
the conventions it set for itself. However, I can provide a link to the
library for reference on the issue if that would be helpful for people that
are contributing to the design aspects of this RFC.

The current assumption is that a Number always holds a single value. How
if we made it so that it held two values? They are the numerator and the
denominator.

Again, my experience on the issue is with the development of my own
library on the issue, however in my case I fully separated that kind of
object into its own class Fraction, and gave the kinds of operations
we've been discussing to the class Decimal. Storing numerators and
denominators for as long as possible involves a completely different set of
math. For instance, you need an algorithm to determine the Greatest Common
Factor and the Least Common Multiple in such a class, because there are a
lot of places where you would need to find the smallest common denominator
or simplify the fraction.

Abstracting between the Fraction and Decimal so that they worked with
each other honestly introduced the most complex and inscrutable code in my
entire library, so unless fractions are themselves also a design goal of
this RFC, I would recommend against it.

Jordan

An addendum:

Having two classes Fraction and Decimal necessitated that I had a
Number class they both extended, as there are many situations where I
would want to type-hint "anything that calculation can be done on with
arbitrary precision" instead of specifically one or the other. I also
provided the NumberInterface, DecimalInterface, and
FractionInterface, though I don't think that would be necessary here as
this is much more just a wrapper for BCMath than an extension of it. The
main goal of my library was not to act as a wrapper for BCMath, it was to
EXTEND BCMath with additional capabilities, such as trigonometric functions
that have arbitrary precision, so keep that in mind when weighing input of
mine that is referencing the work I have done on this topic. The design
goals were different.

Jordan

11 months ago by Saki Takamachi — view source

unread

Hi Barney, Jordan,

I think that's a sufficiently different data type that it should be a different class (if required), and probably a separate RFC, and for now it's better to stay closer to the existing BCMath API.

Developers should be prepared to accept that an arbitrary precision decimal can't represent 1/3 exactly, just like a binary float can't represent 1/10 exactly.

Again, my experience on the issue is with the development of my own library on the issue, however in my case I fully separated that kind of object into its own class Fraction, and gave the kinds of operations we've been discussing to the class Decimal. Storing numerators and denominators for as long as possible involves a completely different set of math. For instance, you need an algorithm to determine the Greatest Common Factor and the Least Common Multiple in such a class, because there are a lot of places where you would need to find the smallest common denominator or simplify the fraction.

Abstracting between the Fraction and Decimal so that they worked with each other honestly introduced the most complex and inscrutable code in my entire library, so unless fractions are themselves also a design goal of this RFC, I would recommend against it.

Having two classes Fraction and Decimal necessitated that I had a Number class they both extended, as there are many situations where I would want to type-hint "anything that calculation can be done on with arbitrary precision" instead of specifically one or the other. I also provided the NumberInterface, DecimalInterface, and FractionInterface, though I don't think that would be necessary here as this is much more just a wrapper for BCMath than an extension of it. The main goal of my library was not to act as a wrapper for BCMath, it was to EXTEND BCMath with additional capabilities, such as trigonometric functions that have arbitrary precision, so keep that in mind when weighing input of mine that is referencing the work I have done on this topic. The design goals were different.

Agree. I was a little too focused on precision and lost sight of the larger goal.

In my library, if the scale is unspecified, I actually set the scale to 10 OR the length of the input string, including integer decimals, whichever is larger. Since I was designing my own library I could do things like that as convention, and a scale of 10 is extremely fast, even with the horrifically slow BCMath library, but covers most use cases (the overwhelmingly common of which is exact calculation of money).

My library handles scale using the following design. It's not necessarily correct here, as I was designing a PHP library instead of something for core, AND my library does not have to deal with operator overloads so I'm always working with method signatures instead, AND it's possible that my class/method design is inferior to other alternatives, however it went:

Each number constructor allowed for an optional input scale.

The input number was converted into the proper formatting from allowed input types, and then the implicit scale is set to the total number of digits.

If the input scale was provided, the determined scale is set to that value.

Otherwise, the determined scale at construction is set to 10 or the implicit scale of "number of digits", whichever is larger.

The class contained the roundToScale method, which allowed you to provide the desired scale and the rounding method, and then would set the determined scale to that value after rounding. It contained the round method with the same parameters to allow rounding to a specific scale without also setting the internal determined scale at the same time.

The class contained the setScale method which set the value of the internal determined scale value to an int without mutating the value at all.

All mathematical operation methods which depended on scale, (such as div or pow), allowed an optional input scale that would be used for calculation if present. If it was not present, the internal calculations were done by taking the higher of the determined scale between the two operands, and then adding 2, and then the result was done by rounding using the default method of ROUND_HALF_EVEN if no rounding method was provided.

Again, though I have spent a lot of design time on this issue for the math library I developed, my library did not have to deal with the RFC process for PHP or maintain consistency with the conventions of PHP core, only with the conventions it set for itself. However, I can provide a link to the library for reference on the issue if that would be helpful for people that are contributing to the design aspects of this RFC.

The two use cases at issue here are when the div and pow's exponent are negative values. So how about allowing only these two methods to optionally set $scale and $roundMode ?

The constructor takes only $num and always uses implicit scaling. There is no option for the user to specify an arbitrary scale.
$scale: If specified, use that value, otherwise use 10. The scale specified here is added to the scale of the left operand and used as the scale of the result. In other words, (new Number('0.01')->div('3', 2)) results in '0.0030' // scale = 2 + 2 = 4.
$roundMode: Specifies the rounding method when the result does not fit within the scale. The initial value is PHP_ROUND_TOWARD_ZERO, which matches the behavior of the BCMath function. That is, just truncate.
If lucky enough to get the result within the scale, apply the implicit scale to the result. In other words, if calculate 1 / 2, the resulting scale will be 1, even if scale is null or specify a value such as 20 for scale.
The result of a calculation with operator overloading is the same as if the option was not used when executing the method.

However, I'm not sure if naming it $scale is appropriate.

Also, since BCMath\Number is not made into a final class, there is a possibility of implementing an inherited class in userland. Regarding this, is it better to make the calculation method a final method, or to use a function overridden by the user when executing from the opcode?

Regards.

Saki

11 months ago by Jordan LeDoux — view source

unread

The two use cases at issue here are when the div and pow's exponent are
negative values. So how about allowing only these two methods to optionally
set $scale and $roundMode ?

The constructor takes only $num and always uses implicit scaling.
There is no option for the user to specify an arbitrary scale.

$scale: If specified, use that value, otherwise use 10. The scale
specified here is added to the scale of the left operand and used as the
scale of the result. In other words, (new Number('0.01')->div('3', 2))
results in '0.0030' // scale = 2 + 2 = 4.

$roundMode: Specifies the rounding method when the result does not fit
within the scale. The initial value is PHP_ROUND_TOWARD_ZERO, which
matches the behavior of the BCMath function. That is, just truncate.

If lucky enough to get the result within the scale, apply the implicit
scale to the result. In other words, if calculate 1 / 2, the resulting
scale will be 1, even if scale is null or specify a value such as 20
for scale.

The result of a calculation with operator overloading is the same as if
the option was not used when executing the method.

However, I'm not sure if naming it $scale is appropriate.

Also, since BCMath\Number is not made into a final class, there is a
possibility of implementing an inherited class in userland. Regarding this,
is it better to make the calculation method a final method, or to use a
function overridden by the user when executing from the opcode?

The issue is that, presumably, this method will be used within the operator
overload portion of the class entry in C. If it is allowed to be
overridden, then this RFC is sort of providing a stealth operator overload
to PHP developers. As much as I am for operator overloads having written an
RFC for it, and as much as I find the arguments generally against it
lacking, I am not in favor of doing it that way with a kind of... unspoken
capability to overload the basic math operators in userland. I very much
like the feature, but I also think it should be intentionally and
specifically designed, which is why I spent a long time on it. I do not get
a vote for RFCs, but I would vote against this if I could just for that
reason IF the calculation methods were not private, the class was not
final, AND the function entry was used in the operator overload.

And operator overloads are also the place where what you outlined above
gets murky. I think what you outlined is very close to a good final design
for just the method usage side, but the operator usage side CANNOT provide
a scale or a rounding mode. That should be taken into consideration,
because allowing this object to be used with operators is probably the
single largest benefit this RFC will provide to PHP developers.

What I ended up doing was that the VALUE of the object was immutable, but
the other information was not immutable. That has its own downsides, but
does allow for very explicit control from the developer at the section of
code using the class, but also avoids creating copies of the object or
instantiating a new object for every single "setting" change during
calculations.

Jordan

11 months ago by Jordan LeDoux — view source

unread

On Tue, Apr 2, 2024 at 5:05 PM Jordan LeDoux jordan.ledoux@gmail.com
wrote:

The two use cases at issue here are when the div and pow's exponent are
negative values. So how about allowing only these two methods to optionally
set $scale and $roundMode ?

The constructor takes only $num and always uses implicit scaling.
There is no option for the user to specify an arbitrary scale.

$scale: If specified, use that value, otherwise use 10. The scale
specified here is added to the scale of the left operand and used as the
scale of the result. In other words, (new Number('0.01')->div('3', 2))
results in '0.0030' // scale = 2 + 2 = 4.

$roundMode: Specifies the rounding method when the result does not
fit within the scale. The initial value is PHP_ROUND_TOWARD_ZERO, which
matches the behavior of the BCMath function. That is, just truncate.

If lucky enough to get the result within the scale, apply the implicit
scale to the result. In other words, if calculate 1 / 2, the resulting
scale will be 1, even if scale is null or specify a value such as 20
for scale.

The result of a calculation with operator overloading is the same as if
the option was not used when executing the method.

However, I'm not sure if naming it $scale is appropriate.

Also, since BCMath\Number is not made into a final class, there is a
possibility of implementing an inherited class in userland. Regarding this,
is it better to make the calculation method a final method, or to use a
function overridden by the user when executing from the opcode?

The issue is that, presumably, this method will be used within the
operator overload portion of the class entry in C. If it is allowed to be
overridden, then this RFC is sort of providing a stealth operator overload
to PHP developers. As much as I am for operator overloads having written an
RFC for it, and as much as I find the arguments generally against it
lacking, I am not in favor of doing it that way with a kind of... unspoken
capability to overload the basic math operators in userland. I very much
like the feature, but I also think it should be intentionally and
specifically designed, which is why I spent a long time on it. I do not get
a vote for RFCs, but I would vote against this if I could just for that
reason IF the calculation methods were not private, the class was not
final, AND the function entry was used in the operator overload.

And operator overloads are also the place where what you outlined above
gets murky. I think what you outlined is very close to a good final design
for just the method usage side, but the operator usage side CANNOT provide
a scale or a rounding mode. That should be taken into consideration,
because allowing this object to be used with operators is probably the
single largest benefit this RFC will provide to PHP developers.

What I ended up doing was that the VALUE of the object was immutable, but
the other information was not immutable. That has its own downsides, but
does allow for very explicit control from the developer at the section of
code using the class, but also avoids creating copies of the object or
instantiating a new object for every single "setting" change during
calculations.

Jordan

I should clarify, the portion of your outline that I feel is not sufficient
for the operator overload use case is that there is no way to use both
operator overloads AND a scale other than 10 + left operand scale.

Jordan

11 months ago by Saki Takamachi — view source

unread

Hi Jordan,

The issue is that, presumably, this method will be used within the operator overload portion of the class entry in C. If it is allowed to be overridden, then this RFC is sort of providing a stealth operator overload to PHP developers. As much as I am for operator overloads having written an RFC for it, and as much as I find the arguments generally against it lacking, I am not in favor of doing it that way with a kind of... unspoken capability to overload the basic math operators in userland. I very much like the feature, but I also think it should be intentionally and specifically designed, which is why I spent a long time on it. I do not get a vote for RFCs, but I would vote against this if I could just for that reason IF the calculation methods were not private, the class was not final, AND the function entry was used in the operator overload.

And operator overloads are also the place where what you outlined above gets murky. I think what you outlined is very close to a good final design for just the method usage side, but the operator usage side CANNOT provide a scale or a rounding mode. That should be taken into consideration, because allowing this object to be used with operators is probably the single largest benefit this RFC will provide to PHP developers.

What I ended up doing was that the VALUE of the object was immutable, but the other information was not immutable. That has its own downsides, but does allow for very explicit control from the developer at the section of code using the class, but also avoids creating copies of the object or instantiating a new object for every single "setting" change during calculations.

Agree. I also have negative thoughts about this, but I wanted to hear everyone's opinions, so I sent the email I mentioned earlier.

If make a class final, users will not be able to add arbitrary methods, so I think making each method final. Although it is possible to completely separate behavior between method and opcode calculations, this is inconsistent and confusing to users and should be avoided.

I should clarify, the portion of your outline that I feel is not sufficient for the operator overload use case is that there is no way to use both operator overloads AND a scale other than 10 + left operand scale.

How about setting the $scale and $roundMode that I mentioned earlier in the constructor instead of div and pow? By doing so, we can use any scale when calculating with opcodes. If we want to specify a different scale for each calculation, you can do it using methods.
Or would it be more convenient to have a method to reset $scale and $roundMode? It is immutable, so when reset it returns a new instance.

Regards.

Saki

11 months ago by Saki Takamachi — view source

unread

PS: Yep, this is pretty much Jordan's library idea.

Saki

11 months ago by Barney Laurance — view source

unread

If make a class final, users will not be able to add arbitrary methods, so I think making each method final.

Right, if a class is not final but has only final methods (including
constructor) and no protected properties then it allows people to extend
it but not break any encapsulation or (I think) impose any more BC
requirements on the maintainer (unless we care about conflicting with
method names of child classes when updating). It lets people just do
something a bit like adding 'extension methods' in C#. I like it. People
could write e.g. $bcChildNumber->isPrime() instead of
BcNumberUtils::isPrime($bcNumber)

11 months ago by Jordan LeDoux — view source

unread

If make a class final, users will not be able to add arbitrary methods, so
I think making each method final. Although it is possible to completely
separate behavior between method and opcode calculations, this is
inconsistent and confusing to users and should be avoided.

Right, if a class is not final but has only final methods (including

constructor) and no protected properties then it allows people to extend it
but not break any encapsulation or (I think) impose any more BC
requirements on the maintainer (unless we care about conflicting with
method names of child classes when updating). It lets people just do
something a bit like adding 'extension methods' in C#. I like it. People
could write e.g. $bcChildNumber->isPrime() instead of
BcNumberUtils::isPrime($bcNumber)

Yeah, I suspect the most common child class will be something like "Money"
that additionally has a parameter denoting what currency the value is in,
since that is by far the most common use case for arbitrary precision math
in PHP. Making the calculation methods final should do fine in my opinion.

How about setting the $scale and $roundMode that I mentioned earlier
in the constructor instead of div and pow? By doing so, we can use any
scale when calculating with opcodes. If we want to specify a different
scale for each calculation, you can do it using methods.
Or would it be more convenient to have a method to reset $scale and
$roundMode? It is immutable, so when reset it returns a new instance.

I think that having it be an optional part of the constructor and having a
method to modify it is probably sufficient to help with operator overloads.
Importantly, it still needs to take that setting and use it intelligently
depending on the operation as we've discussed, but that should cover the
use cases I was imagining.

The exact behavior should be very clearly documented and explained though,
as this is a different way of interacting with BCMath than the functions.
It's better I think, because it takes advantage of the fact that this will
be state-aware, which the functions aren't, and is one of the largest
benefits of using objects.

Also, to address an earlier question you had Saki, the term used should be
"scale". Scale is the total number of digits to the right of the decimal
point. Precision is the total number of significant digits. The BCMath C
library works on scale, I believe, so that should carry over here. Most
other arbitrary precision libraries in C use precision.

Jordan

11 months ago by Saki Takamachi — view source

unread

Hi Barney, Jordan,

Right, if a class is not final but has only final methods (including constructor) and no protected properties then it allows people to extend it but not break any encapsulation or (I think) impose any more BC requirements on the maintainer (unless we care about conflicting with method names of child classes when updating). It lets people just do something a bit like adding 'extension methods' in C#. I like it. People could write e.g. $bcChildNumber->isPrime() instead of BcNumberUtils::isPrime($bcNumber)

Yeah, I suspect the most common child class will be something like "Money" that additionally has a parameter denoting what currency the value is in, since that is by far the most common use case for arbitrary precision math in PHP. Making the calculation methods final should do fine in my opinion.

I realize that the purpose of using final here is to prevent users from customizing the behavior of operator overloads. In the RFC, there are methods that do not involve operator overloading, such as round, but do you think these should also be made final? Or should only methods related to overloading be made final?

IMHO, there are methods such as powmod that are computational functions but are not related to operator overloading, and from the viewpoint of consistency, we believe that all methods should be made final without exception.

I think that having it be an optional part of the constructor and having a method to modify it is probably sufficient to help with operator overloads. Importantly, it still needs to take that setting and use it intelligently depending on the operation as we've discussed, but that should cover the use cases I was imagining.

The exact behavior should be very clearly documented and explained though, as this is a different way of interacting with BCMath than the functions. It's better I think, because it takes advantage of the fact that this will be state-aware, which the functions aren't, and is one of the largest benefits of using objects.

Also, to address an earlier question you had Saki, the term used should be "scale". Scale is the total number of digits to the right of the decimal point. Precision is the total number of significant digits. The BCMath C library works on scale, I believe, so that should carry over here. Most other arbitrary precision libraries in C use precision.

Thanks, I agree with you that it works a little differently than the existing BCMath function, so it needs to be well-documented.

I have summarized the discussion so far, made some corrections, and updated the RFC. I would be happy if you could check it out.

Regards.

Saki

11 months ago by Jordan LeDoux — view source

unread

Thanks, I agree with you that it works a little differently than the
existing BCMath function, so it needs to be well-documented.

I have summarized the discussion so far, made some corrections, and
updated the RFC. I would be happy if you could check it out.

It looks pretty good. It should probably give specific guidelines on how
"no scale provided" works for addition, subtraction, multiplication,
division, and exponentiation. The section now doesn't make it clear that
the returned scale will behave differently for addition (the current
example) than for division.

Jordan

11 months ago by Saki Takamachi — view source

unread

Hi Jordan,

It looks pretty good. It should probably give specific guidelines on how "no scale provided" works for addition, subtraction, multiplication, division, and exponentiation. The section now doesn't make it clear that the returned scale will behave differently for addition (the current example) than for division.

I see, I'll add that later, thanks!

Regards.

Saki

11 months ago by Saki Takamachi — view source

unread

Hi,

I added some examples, corrected incorrect explanations, etc. Now that the specifications have been fairly finalized and the RFC has been clarified, I would like everyone to check it out again.

https://wiki.php.net/rfc/support_object_type_in_bcmath

Regards.

Saki

11 months ago by Christian Schneider — view source

unread

Am 04.04.2024 um 08:29 schrieb Saki Takamachi saki@sakiot.com:

I added some examples, corrected incorrect explanations, etc. Now that the specifications have been fairly finalized and the RFC has been clarified, I would like everyone to check it out again.

https://wiki.php.net/rfc/support_object_type_in_bcmath

Looks very promising!

The default value for rounding is PHP_ROUND_TOWARD_ZERO. Is this because it is the same as the BCMath functions for consistency only or do you expect this to be the most used rounding mode?
I didn't find any discussion about this choice, sorry if I missed it.

Regards,

Chris

11 months ago by Saki Takamachi — view source

unread

Hi Chris,

Looks very promising!

The default value for rounding is PHP_ROUND_TOWARD_ZERO. Is this because it is the same as the BCMath functions for consistency only or do you expect this to be the most used rounding mode?
I didn't find any discussion about this choice, sorry if I missed it.

Thank you!

Yes, you're right, I've made it match the existing behavior of BCMath. And rest assured, there has been no discussion about that yet.

I haven't thought about it in detail yet, but maybe there are other ideal initial values?

Regards.

Saki

11 months ago by tim@bastelstu.be — view source

unread

Hi

[cleaning out CC list]

I added some examples, corrected incorrect explanations, etc. Now that the specifications have been fairly finalized and the RFC has been clarified, I would like everyone to check it out again.

https://wiki.php.net/rfc/support_object_type_in_bcmath

Can you please clean up the RFC text? I find the “Updated X/X” bits
confusing, because I'm not sure whether this is what the RFC is going to
propose, or whether this is something that was superseded by the
following discussion.

The Wiki already provides a version history should folks be interested
in what has actually changed.

Another tip regarding Wiki syntax: By using <PHP> instead of <code> the
code examples will get syntax highlighting, making them more readable.

Best regards
Tim Düsterhus

11 months ago by Saki Takamachi — view source

unread

Hi Tim,

Can you please clean up the RFC text? I find the “Updated X/X” bits confusing, because I'm not sure whether this is what the RFC is going to propose, or whether this is something that was superseded by the following discussion.

The Wiki already provides a version history should folks be interested in what has actually changed.

Another tip regarding Wiki syntax: By using <PHP> instead of <code> the code examples will get syntax highlighting, making them more readable.

Thank you for teaching me. I will fix it later!

Regards.

Saki

11 months ago by Barney Laurance — view source

unread

If make a class final, users will not be able to add arbitrary
methods, so I think making each method final. Although it is
possible to completely separate behavior between method and opcode
calculations, this is inconsistent and confusing to users and
should be avoided.


Right, if a class is not final but has only final methods
(including constructor) and no protected properties then it allows
people to extend it but not break any encapsulation or (I think)
impose any more BC requirements on the maintainer (unless we care
about conflicting with method names of child classes when
updating). It lets people just do something a bit like adding
'extension methods' in C#. I like it. People could write e.g.
`$bcChildNumber->isPrime()` instead of
`BcNumberUtils::isPrime($bcNumber)`

Yeah, I suspect the most common child class will be something like
"Money" that additionally has a parameter denoting what currency the
value is in, since that is by far the most common use case for
arbitrary precision math in PHP. Making the calculation methods final
should do fine in my opinion.

I don't think it will be possible to make a good Money class as a child
of this. BcNum is a read-only class, so if the constructor of BcNum is
final then the child class won't be able to take the currency as a
constructor param, and won't be able to protect the invariant that
currency must be initialized. If the constructor of BcNum were made
non-final then BcNum wouldn't be able to protect the invariant of the
numeric value always being initialized once the constructor has
returned. And it should be straightforward to make a good money class as
a composition of BcNum and a currency enum or string.

There's probably not any good use case for a subclass to add properties,
unless perhaps the subclass developers are willing do do away with some
of the checks that would normally be done on a value object by the PHP
runtime to keep it valid (maybe replacing them with static analysis
checks). But there are lots of reasonable use cases for subclasses to
add methods, even if they're effectively syntax sugar for static methods
with a BcNum typed param.

Having just written that I've now checked the behavior of DateTime - see
https://3v4l.org/5DQZg . The constructor of that isn't final, so a child
class can replace it with an empty constructor. But if it does that and
leaves the value uninitialized then it blows up on a call to format.
That's probably not something we should emulate. DateTimeImmutable
behaves the same way.

11 months ago by Jordan LeDoux — view source

unread

On Thu, Apr 4, 2024 at 2:19 PM Barney Laurance barney@redmagic.org.uk
wrote:

I don't think it will be possible to make a good Money class as a child of
this. BcNum is a read-only class, so if the constructor of BcNum is final
then the child class won't be able to take the currency as a constructor
param, and won't be able to protect the invariant that currency must be
initialized. If the constructor of BcNum were made non-final then BcNum
wouldn't be able to protect the invariant of the numeric value always being
initialized once the constructor has returned. And it should be
straightforward to make a good money class as a composition of BcNum and a
currency enum or string.

The RFC does not list the constructor as final, and I would not expect it
to.

There's probably not any good use case for a subclass to add properties,
unless perhaps the subclass developers are willing do do away with some of
the checks that would normally be done on a value object by the PHP runtime
to keep it valid (maybe replacing them with static analysis checks). But
there are lots of reasonable use cases for subclasses to add methods, even
if they're effectively syntax sugar for static methods with a BcNum typed
param.

I literally just provided in the quote you are replying to a good use case
for a subclass. You can do the same thing with composition yeah, and that
might even be better to a lot of people, but I don't think this RFC should
take a position AGAINST subclasses and in favor of composition.

Having just written that I've now checked the behavior of DateTime - see
https://3v4l.org/5DQZg . The constructor of that isn't final, so a child
class can replace it with an empty constructor. But if it does that and
leaves the value uninitialized then it blows up on a call to format.
That's probably not something we should emulate. DateTimeImmutable behaves
the same way.

Why not? Writing something like that obviously does not work, and the error
would be immediately apparent. Is it possible to create something that will
error? Of course. That's not an error that needs to be aggressively guarded
against, because the feedback is rapid and obvious.

Jordan

11 months ago by Saki Takamachi — view source

unread

Hi Barney, Jordan,

I would like to thank you all again for discussing this RFC in great detail. I believe that the specification for the BCMath\Number class is in very good shape due to the detailed discussions and the great amount of feedback. Now that we have been able to brush up the specifications so far, I would be very happy if we could have a good discussion about the concerns that still exist.

Coming back to this point, I think these are basic features that people would expect to be there - I think I would find just slightly frustrating to start learning how to use a class like this and then
find that it doesn't have these functions. Casting and calling intval or floatval all feel like slightly awkward workarounds that shouldn't be needed in a greenfield project. We know that the string
inside the object is always a numeric string, so it should be easier to parse it as an int than to parse it as a date or a JSON document. Code doing the latter should stand out as odd looking.

The class cannot guarantee that it can return a value in the type you request however, so the way that is handled would need to be decided. The value can easily be outside of the range of an int. Should it return a float silently in that case for toInt()? What if the value is beyond the range of a float? That would be a very rare situation, as floats can represent extremely large numbers (with very reduced accuracy), but I would expect it to throw an exception if that happened. Ideally an exception that I could catch and ignore, since I can almost surely deal with that error in most situations.

What about a number that is so small that it can't fit in a float? Similar situation, though I expect it would occur slightly more often than a number being too large to fit in a float, even though it would also be rare.

I think these helper functions belong in the RFC, but they aren't quite straightforward, which is what I think Saki was alluding to.

Yes I agree there are subtleties to work out for these functions. I don't think there's such a thing as a number to small to fit in a float. Converting from decimal to float is always an approximation, sometimes the best approximation available as a float will be either 0 or -0.

If there is a demand for such functionality, I think it should of course be included in the RFC. However, as Jordan mentioned, proper error handling is required. IMHO, I think the following specifications are good:

toInt(): If it has a decimal part, round it to an integer according to the rounding mode. If the result is too large to be represented as an int, it will not be converted to a float and will throw an exception meaning it cannot be converted to an int.
toFloat(): If the result is too large or too small, throws an exception meaning it cannot be converted, just like int. Also, if the value is within the representable range but the precision is too fine, such as 1.0000000000000001, it will be rounded according to the rounding mode. However, if round to a precision of 17 or 18 digits, the error will end up being very large, so refer to the value defined in PHP_FLOAT_DIG and use 15 digits as the maximum number of digits. This constant is 16 for IBM and 15 for others, but for consistency of operation it should be set to 15.

I don't think it will be possible to make a good Money class as a child of this. BcNum is a read-only class, so if the constructor of BcNum is final then the child class won't be able to take the currency as a constructor param, and won't be able to protect the invariant that currency must be initialized. If the constructor of BcNum were made non-final then BcNum wouldn't be able to protect the invariant of the numeric value always being initialized once the constructor has returned. And it should be straightforward to make a good money class as a composition of BcNum and a currency enum or string.

The RFC does not list the constructor as final, and I would not expect it to.

There's probably not any good use case for a subclass to add properties, unless perhaps the subclass developers are willing do do away with some of the checks that would normally be done on a value object by the PHP runtime to keep it valid (maybe replacing them with static analysis checks). But there are lots of reasonable use cases for subclasses to add methods, even if they're effectively syntax sugar for static methods with a BcNum typed param.

I literally just provided in the quote you are replying to a good use case for a subclass. You can do the same thing with composition yeah, and that might even be better to a lot of people, but I don't think this RFC should take a position AGAINST subclasses and in favor of composition.

Having just written that I've now checked the behavior of DateTime - see https://3v4l.org/5DQZg. The constructor of that isn't final, so a child class can replace it with an empty constructor. But if it does that and leaves the value uninitialized then it blows up on a call to format. That's probably not something we should emulate. DateTimeImmutable behaves the same way.

Why not? Writing something like that obviously does not work, and the error would be immediately apparent. Is it possible to create something that will error? Of course. That's not an error that needs to be aggressively guarded against, because the feedback is rapid and obvious.

I don't think protecting initialization by the constructor is necessary. https://3v4l.org/YroTN
This problem can occur with any class, not just DateTime. And I think this is the responsibility of the user's code, and the language just needs to throw an exception appropriately.

To clarify: In my opinion, I think that value objects provided by languages are not intended to force immutability or design onto users, to provide functionality to users who expect them to behave as immutable value objects. Therefore, even if a child class that inherits from Number behaves in a way that is not immutable, it is the responsibility of the user and there is no need to prohibit it.

In other words, the concerns we are currently aware of are issues of responsibility on the user's side, and my opinion is that there is no need to give that much consideration. However, this is my personal opinion, and there may be more reasonable opinions.

Regards.

Saki

11 months ago by Barney Laurance — view source

unread

I don't think it will be possible to make a good Money class as a child of this. BcNum is a read-only class, so if the constructor of BcNum is final then the child class won't be able to take the currency as a constructor param, and won't be able to protect the invariant that currency must be initialized. If the constructor of BcNum were made non-final then BcNum wouldn't be able to protect the invariant of the numeric value always being initialized once the constructor has returned. And it should be straightforward to make a good money class as a composition of BcNum and a currency enum or string.
The RFC does not list the constructor as final, and I would not expect it to.
There's probably not any good use case for a subclass to add properties, unless perhaps the subclass developers are willing do do away with some of the checks that would normally be done on a value object by the PHP runtime to keep it valid (maybe replacing them with static analysis checks). But there are lots of reasonable use cases for subclasses to add methods, even if they're effectively syntax sugar for static methods with a BcNum typed param.
I literally just provided in the quote you are replying to a good use case for a subclass. You can do the same thing with composition yeah, and that might even be better to a lot of people, but I don't think this RFC should take a position AGAINST subclasses and in favor of composition.
Having just written that I've now checked the behavior of DateTime - seehttps://3v4l.org/5DQZg. The constructor of that isn't final, so a child class can replace it with an empty constructor. But if it does that and leaves the value uninitialized then it blows up on a call to format. That's probably not something we should emulate. DateTimeImmutable behaves the same way.
Why not? Writing something like that obviously does not work, and the error would be immediately apparent. Is it possible to create something that will error? Of course. That's not an error that needs to be aggressively guarded against, because the feedback is rapid and obvious.

I don't think protecting initialization by the constructor is necessary.https://3v4l.org/YroTN
This problem can occur with any class, not just DateTime. And I think this is the responsibility of the user's code, and the language just needs to throw an exception appropriately.

To clarify: In my opinion, I think that value objects provided by languages are not intended to force immutability or design onto users, to provide functionality to users who expect them to behave as immutable value objects. Therefore, even if a child class that inherits from Number behaves in a way that is not immutable, it is the responsibility of the user and there is no need to prohibit it.

In other words, the concerns we are currently aware of are issues of responsibility on the user's side, and my opinion is that there is no need to give that much consideration. However, this is my personal opinion, and there may be more reasonable opinions.

Ah ok. Maybe that's the standard way internal classes are written, and
we can consider it generally an error for a child class to override the
constructor without calling parent::__construct(). I know tools warn
users not to forget the parent call.

And I had been thinking of BcNum as a readonly class, but I see it's not
an readonly class, it's a mutable class with its one and only property
being readonly. I agree that it seems unnecessary to force child
classes to be immutable - I wasn't concerned about them adding
additional, mutable, properties, only about leaving the value property
uninitialized. (Btw I would have supported the 1st part of the 2022
Readonly amendments RFC, which I think would have made a readonly class
behave the same as a class where every property is readonly,
particularly since would have allowed implementing the example given on
the Wikipedia page for LSP: a circle with fixed centre and mutable
radius may inherit from an immutable point)

11 months ago by Saki Takamachi — view source

unread

Hi Barney,

Ah ok. Maybe that's the standard way internal classes are written, and we can consider it generally an error for a child class to override the constructor without calling parent::__construct(). I know tools warn users not to forget the parent call.

Thanks, A fallback would be possible to always set the initial value to 0, but an error would probably be easier to understand.

And I had been thinking of BcNum as a readonly class, but I see it's not an readonly class, it's a mutable class with its one and only property being readonly. I agree that it seems unnecessary to force child classes to be immutable - I wasn't concerned about them adding additional, mutable, properties, only about leaving the value property uninitialized. (Btw I would have supported the 1st part of the 2022 Readonly amendments RFC, which I think would have made a readonly class behave the same as a class where every property is readonly, particularly since would have allowed implementing the example given on the Wikipedia page for LSP: a circle with fixed centre and mutable radius may inherit from an immutable point)

Ah, sorry about that, I completely forgot about the readonly class! In that case, it might make sense to make the classes completely immutable, since users are already accustomed to using such classes.

This is a departure from what I said in my previous email, but how do you think about making it a read-only class?

Regards.

Saki

11 months ago by Barney Laurance — view source

unread

Hi Barney,

Ah ok. Maybe that's the standard way internal classes are written, and we can consider it generally an error for a child class to override the constructor without calling parent::__construct(). I know tools warn users not to forget the parent call.
Thanks, A fallback would be possible to always set the initial value to 0, but an error would probably be easier to understand.

Uninitialized is miles better than 0 I think. 0 is a meaningful number
just like any other and we should very strictly avoid inserting made up
numbers into people's applications. Let them fail fast, not output fake
data. I'd rather my web shop crashes than gives things away for free.

And I had been thinking of BcNum as a readonly class, but I see it's not an readonly class, it's a mutable class with its one and only property being readonly. I agree that it seems unnecessary to force child classes to be immutable - I wasn't concerned about them adding additional, mutable, properties, only about leaving the value property uninitialized. (Btw I would have supported the 1st part of the 2022 Readonly amendments RFC, which I think would have made a readonly class behave the same as a class where every property is readonly, particularly since would have allowed implementing the example given on the Wikipedia page for LSP: a circle with fixed centre and mutable radius may inherit from an immutable point)
Ah, sorry about that, I completely forgot about the readonly class! In that case, it might make sense to make the classes completely immutable, since users are already accustomed to using such classes.

This is a departure from what I said in my previous email, but how do you think about making it a read-only class?

Tim has convinced me that it should be a final class, in which case
there's no meaningful distinction between a readonly class and a class
with no mutable properties. In that case just for simplicity I'd say it
should be a readonly class.

If it's not a final class I think I'm not the right person to ask, since
as I said I don't really like the fact that a readonly class behaves any
differently to a class with no mutable properties. I prefer the behavior
of the latter.

11 months ago by Saki Takamachi — view source

unread

Hi Tim, Barney,

Your Money example would allow for unsound and/or non-sense behavior, such as:

$fiveEuros = new Money(5, 'EUR');
$tenDollars = new Money(10, 'EUR');

$what = $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

I've already sent a sibling email, explaining why I believe that making the Number class not final is a mistake. However I'd also like to comment on that specific bit of your email:

I strongly believe in misuse-resistant APIs. Users should generally be steered towards making the right choice, even without needing to consult the documentation. For example, by making the "right choice" the easiest choice or by preventing "wrong choices" entirely.

Preventing folks from making wrong choices is overall less costly than them realizing that they made a wrong choice and then being unable to change it, due to backwards compatibility or interoperability concerns.

PHP has enough gotchas as it is, so for any new API making it a great API, not just an okay API should be part of the consideration. APIs within PHP need to survive for 10+ years.

Thanks for that very important point, Tim. I was designing classes with GMP in mind, so I overlooked the point you mentioned.

For reference, classes that inherit from GMP return GMP.

Uninitialized is miles better than 0 I think. 0 is a meaningful number just like any other and we should very strictly avoid inserting made up numbers into people's applications. Let them fail fast, not output fake data. I'd rather my web shop crashes than gives things away for free.

Tim has convinced me that it should be a final class, in which case there's no meaningful distinction between a readonly class and a class with no mutable properties. In that case just for simplicity I'd say it should be a readonly class.

If it's not a final class I think I'm not the right person to ask, since as I said I don't really like the fact that a readonly class behaves any differently to a class with no mutable properties. I prefer the behavior of the latter.

Due to the points Tim mentioned, I decided to make BCMath\Number a final class. And, as you say, for clarity's sake I would make it a read-only class.

This also means that we don't have to worry about errors due to values not being set in the constructor.

Regards.

Saki

11 months ago by tim@bastelstu.be — view source

unread

Hi

In other words, the concerns we are currently aware of are issues of responsibility on the user's side, and my opinion is that there is no need to give that much consideration. However, this is my personal opinion, and there may be more reasonable opinions.

I've already sent a sibling email, explaining why I believe that making
the Number class not final is a mistake. However I'd also like to
comment on that specific bit of your email:

I strongly believe in misuse-resistant APIs. Users should generally be
steered towards making the right choice, even without needing to consult
the documentation. For example, by making the "right choice" the easiest
choice or by preventing "wrong choices" entirely.

Preventing folks from making wrong choices is overall less costly than
them realizing that they made a wrong choice and then being unable to
change it, due to backwards compatibility or interoperability concerns.

PHP has enough gotchas as it is, so for any new API making it a great
API, not just an okay API should be part of the consideration. APIs
within PHP need to survive for 10+ years.

Best regards
Tim Düsterhus

11 months ago by tim@bastelstu.be — view source

unread

Hi

If make a class final, users will not be able to add arbitrary methods, so
I think making each method final. Although it is possible to completely
separate behavior between method and opcode calculations, this is
inconsistent and confusing to users and should be avoided.

Right, if a class is not final but has only final methods (including

constructor) and no protected properties then it allows people to extend it
but not break any encapsulation or (I think) impose any more BC
requirements on the maintainer (unless we care about conflicting with
method names of child classes when updating). It lets people just do
something a bit like adding 'extension methods' in C#. I like it. People
could write e.g. $bcChildNumber->isPrime() instead of
BcNumberUtils::isPrime($bcNumber)

Yeah, I suspect the most common child class will be something like "Money"
that additionally has a parameter denoting what currency the value is in,
since that is by far the most common use case for arbitrary precision math
in PHP. Making the calculation methods final should do fine in my opinion.

Your Money example would allow for unsound and/or non-sense behavior,
such as:

 $fiveEuros = new Money(5, 'EUR');
 $tenDollars = new Money(10, 'EUR');

 $what = $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

Best regards
Tim Düsterhus

11 months ago by Barney Laurance — view source

unread

Your Money example would allow for unsound and/or non-sense
behavior, such as:

    $fiveEuros = new Money(5, 'EUR');
    $tenDollars = new Money(10, 'EUR');

    $what = $fiveEuros + $tenDollars;

What would you expect to be in $what? A BcMath\Number(15)?

Yep, since the add method is final and will return a Number then
Number(15) is the only possible result. And even if you did (new Money(5, 'EUR')) + (new Money(15, 'EUR')), the result would be the
Number 15, not a money. Equally if it was extended to add the isPrime
method as soon as you do any calculation on it you'd get a number
without the isPrime method, so it would be of very limited use. That
could be avoided by returning static instead of self, but that's
makes addition non-commutative which would be confusing, and using new static sort of requires the constructor to be final, which would make
the Money case impossible.

So yes I'd say you've convinced me that the Number class does need to be
a Final class.

11 months ago by Jordan LeDoux — view source

unread

Hi

Your Money example would allow for unsound and/or non-sense behavior,
such as:
 $fiveEuros = new Money(5, 'EUR');
 $tenDollars = new Money(10, 'EUR');

 $what = $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 = $val1->getCurrency();
$val2Currency = $val2->getCurrency();
$val1 = $val1->convertToCommonCurrency();
$val2 = $val2->convertToCommonCurrency();
// perform the necessary calculations using operators
$answer = $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

11 months ago by Saki Takamachi — view source

unread

11 months ago by tim@bastelstu.be — view source

unread

Hi

The only solution I can think of at the moment is to impose the constraint that
when computing operator overloading, if the operands are both objects, they must
be of the exact same class.

Even that would allow for confusing behavior:

 class MyNumber extends Number {
     private $importantMetadata;

     public function doSomething() {
         $this->importantMetadata = random_int(1, 100);
     }
 }

 $a = new MyNumber(5);
 $a->doSomething();
 $b = new MyNumber(10);
 $b->doSomething();

 $what = $a + $b;

What should be the value of $what->importantMetadata be? The property is
private, so as a user adding two of MyNumber would not even be able to
manually fix it up with the correct value, thus requiring the addition
of a "fixup" method that fixes the internal state of the object,
possibly even lacking the necessary information to properly fix up the
state, because it does not know which operations lead to the state.

Best regards
Tim Düsterhus

11 months ago by Jordan LeDoux — view source

unread

Hi

The only solution I can think of at the moment is to impose the
constraint that
when computing operator overloading, if the operands are both objects,
they must
be of the exact same class.

Even that would allow for confusing behavior:
 class MyNumber extends Number {
     private $importantMetadata;

     public function doSomething() {
         $this->importantMetadata = random_int(1, 100);
     }
 }

 $a = new MyNumber(5);
 $a->doSomething();
 $b = new MyNumber(10);
 $b->doSomething();

 $what = $a + $b;
What should be the value of $what->importantMetadata be? The property is
private, so as a user adding two of MyNumber would not even be able to
manually fix it up with the correct value, thus requiring the addition
of a "fixup" method that fixes the internal state of the object,
possibly even lacking the necessary information to properly fix up the
state, because it does not know which operations lead to the state.

Best regards
Tim Düsterhus

That is an absurd example. Why would anyone use inheritance for that class
design? If what you are arguing is "if you look at use cases where
composition is clearly the correct choice then inheritance causes
problems", then I'm not sure what the point of the discussion is.

Jordan

11 months ago by Saki Takamachi — view source

unread

11 months ago by Saki Takamachi — view source

unread

postscript:

For example, Ruby's operator overloading is not by design commutative, so in this case it's always possible to prioritize the properties of the left operand, but which can be a bit confusing.

11 months ago by Saki Takamachi — view source

unread

I'll try to think of some more good ideas, but if you have any suggestions, please let me know.

Regarding $roundMode, a possible approach is to clarify the priority of the current eight values, and if they are different, apply the one with the higher priority.

With that in mind, the question that remains is which of the following three elements should I give up on?

User freedom (inheritability)
Commutativity of computation
Simplicity

Giving up on 1 means adopting a final class. Giving up 2 means always prefering the left operand to determine the properties of the result. (However, I don't really like giving up 2)

In this email, I will explain the approach if you give up on 3. This is an approach I just came up with earlier.

Take a look at the methods shown below:

protected static function resultPropertyRules(string $propertyName, mixed $value1, mixed $value2): mixed {}

This method determines which operand value to use in the result after calculation for a property that the user has defined by extending the class. Calling this in the Number class is pointless and will throw an exception. Child classes may not necessarily need to override this.

Here are the details. These examples assume that all calculations are performed between objects of classes that inherit from Number. Additionally, "properties" in the description refer to properties defined by the user.

Objects to be calculated must be a parent-child relationship or be of the same class.
If they are different objects (parent-child relationship), the calculation result will always be the class of the child.
resultPropertyRules() is not called if it is possible to implicitly determine which properties the result should have.
If cannot determine the value of a property that the result should have, call resultPropertyRules() for that property. In other words, resultPropertyRules() is called as many times as there are properties that need to be evaluated.
If resultPropertyRules() needs to be called and resultPropertyRules() is not overridden, an exception will be thrown.
Similarly, if use resultPropertyRules() and the value cannot be determined properly (for example, if it returns a value of a different type), it will also throw an exception.

Supplement: Example when it is possible to determine the value to be used implicitly

If the user has not defined the property
If all property values are exactly the same
For objects in a parent-child relationship, if a certain property exists only on the child side

Points that need discussion

Suppose that in an object with a parent-child relationship, a certain property is defined on the parent side. In that case, which definition should be used for resultPropertyRules(), parent or child?
A more specific case of the above question: What if the visibility of the property defined in the parent is private?

Regards.

Saki

11 months ago by Rowan Tommins [IMSoP] — view source

unread

Take a look at the methods shown below:
protected static function resultPropertyRules(string $propertyName,
mixed $value1, mixed $value2): mixed {}
This method determines which operand value to use in the result after
calculation for a property that the user has defined by extending the
class.

While this is an intriguing idea, it only solves a narrow set of use
cases. For instance:

The class might want different behaviour for different operations;
e.g. Money(42, 'USD') + Money(42, 'USD') should give Money(84, 'USD');
but Money(42, 'USD') * Money(42, 'USD') should be an error.
Properties might need to interact with each other; e.g. Distance(2,
'metres') + Distance(2, 'feet') could result in Distance(2.6096,
'metres'); but if you calculate one property at a time, you'll end up
with Distance(4, 'metres'), which is clearly wrong.

The fundamental problem is that it ignores the OOP concept of
encapsulation: how an object stores its internal state should not define
its behaviour. Instead, the object should be able to directly define
behaviour for the operations it supports.

Regards,

--
Rowan Tommins
[IMSoP]

11 months ago by Saki Takamachi — view source

unread

Hi Rowan and folks,

While this is an intriguing idea, it only solves a narrow set of use cases. For instance:

The class might want different behaviour for different operations; e.g. Money(42, 'USD') + Money(42, 'USD') should give Money(84, 'USD'); but Money(42, 'USD') * Money(42, 'USD') should be an error.

Properties might need to interact with each other; e.g. Distance(2, 'metres') + Distance(2, 'feet') could result in Distance(2.6096, 'metres'); but if you calculate one property at a time, you'll end up with Distance(4, 'metres'), which is clearly wrong.
The fundamental problem is that it ignores the OOP concept of encapsulation: how an object stores its internal state should not define its behaviour. Instead, the object should be able to directly define behaviour for the operations it supports.

Thank you for pointing it out. Given what Tim and you mentioned, trying to make BCMath\Number have more than just "a value" is probably the wrong approach. (Except for some properties for rationality of calculation)

Inheritable classes are attractive, but there are too many problems to solve to make them a reality, some of which are probably impossible to solve. I explored various possibilities, but in the end, I think making it a final readonly class is the healthiest option.

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

Regards.

Saki

11 months ago by tim@bastelstu.be — view source

unread

Hi

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

11 months ago by Rowan Tommins [IMSoP] — view source

unread

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.

Personally, I'm not a fan of setting the rounding mode and the "max
expansion scale" on each instance, for the same reason I'm not keen on
having the collation on each instance in Derick's Unicode string draft.

I understand the temptation: specifying it for every operation makes
code more verbose, particularly since it rules out use of $a / $b; while
specifying it as a global or scoped option would make code harder to
reason about.

But I think carrying it around on the instance doesn't really solve
either problem, and creates several new ones:

A program which wants all operations to use the same rounding system
still has to specify the options every time it initialises a value,
which is probably nearly as often as operating on them.
A program which wants different modes at different times will end up
calling $foo->withRoundingMode(RoundingMode::HALF_UP)->div(2), which is
more verbose and probably slower than $foo->div(2, RoundingMode::HALF_UP)
You can't look at a function accepting a Number as input and know what
rounding mode it will operate in, unless it explicitly changes it. It
would be easier to scan up to find a per-file / per-block declare()
directive, than to trace the calling code to know the rounding mode of
an instance.
A complex set of rules is invented to "prioritise" the options in
operations like $a + $b. Or, that operation has to be forbidden unless
the mode is consistent, at which point it might as well be a global setting.

As a thought experiment for comparison, imagine if to sort an array
numerically you had to write this:

$array = array_set_sorting_mode($array, SORT_NUMERIC);
$array = array_sort($array);

Or worse, if you had to set it on each string:

$array = array_map($array, fn($s) => $s->withSortingMode(SORT_NUMERIC));
$array = array_sort($array);

Rather than (assuming we replaced the current by-reference sorts):

$array = array_sort($array, SORT_NUMERIC);

Because we're designing an object, attaching extra properties to it is
easy, but I don't think it actually makes it easy to use.

Regards,

--
Rowan Tommins
[IMSoP]

11 months ago by Saki Takamachi — view source

unread

Hi Tim,

The namespace is inconsistently referred to as "BCMath" and "BcMath", please check the entire RFC to unify the casing.

Thanks, I fixed it.

The full “stub” should probably include an explicit "implements Stringable" for clarity.

Agree. I describe it explicitly during implementation.

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.

The property should probably be named "$roundingMode" instead of "$roundMode".

Thank you, I'm not a native speaker, so pointing out English expressions is very helpful. I fixed it.

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.

I missed the point. All defaults have been changed to HALF_UP.

In the stub there's a typo: "puclic", should be "public".

Oops, thanks!

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?

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 see, I wasn't aware of that argument. Removed with().

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?

For the RFC Impact on SAPIs, it should simply be "None", because SAPIs do not need to do anything special.

I fixed it.

For the Backwards Incompatible Changes, it should list that the BcMath\Number class will no longer be available to userland.

I added that.

Thanks!

Saki

11 months ago by Saki Takamachi — view source

unread

Hi Tim,

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.

Once I have a certain amount of content

Sorry.

Saki

11 months ago by tim@bastelstu.be — view source

unread

Hi

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

11 months ago by Saki Takamachi — view source

unread

Hi Tim,

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.
}

Ah I see. I misunderstood. I will add it when the discussion is settled.

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.

Thanks!

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

What I meant was that there is currently no such function in PHP. Do you think I should propose adding it to this RFC?

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.

I agree, but if we adopt Rowan's suggestion this issue will go away, so if the issue is still there when things calm down I'll add the error.

Regards.

Saki

11 months ago by Jordan LeDoux — view source

unread

On Sat, Apr 6, 2024 at 6:45 AM Rowan Tommins [IMSoP] imsop.php@rwec.co.uk
wrote:

Take a look at the methods shown below:
protected static function resultPropertyRules(string $propertyName,
mixed $value1, mixed $value2): mixed {}
This method determines which operand value to use in the result after
calculation for a property that the user has defined by extending the
class.

While this is an intriguing idea, it only solves a narrow set of use
cases. For instance:

The class might want different behaviour for different operations; e.g.
Money(42, 'USD') + Money(42, 'USD') should give Money(84, 'USD'); but
Money(42, 'USD') * Money(42, 'USD') should be an error.

Properties might need to interact with each other; e.g. Distance(2,
'metres') + Distance(2, 'feet') could result in Distance(2.6096, 'metres');
but if you calculate one property at a time, you'll end up with Distance(4,
'metres'), which is clearly wrong.

The fundamental problem is that it ignores the OOP concept of
encapsulation: how an object stores its internal state should not define
its behaviour. Instead, the object should be able to directly define
behaviour for the operations it supports.

If only there had been a feature that carefully considered how all those
things would interact. Oh well.

Okay, then please tell me in which use cases inheritance rather than

composition is the right choice? For the "Money" example, I've already
showcased how it would be broken and I can't think of any example where
inheritance would be superior to composition.

Yes, my example is dumb, but nevertheless it showcased that the native
and unchangeable operations would be unsound for child classes with
custom properties, because the operations would not be able to correctly
fill in the custom properties.

No, like I said, I disagree, but I don't get a vote AND it's not something
I would actually vote no over if I could, so it's not something I'm willing
to put the effort into in that way. I was voicing my disagreement to make
sure it was part of the discussion but I wasn't trying to put the effort
into actually changing the design of the RFC, which I understand is a
little unsatisfactory to some.

As it is, this RFC will be useful to some for sure, but my library will
ignore it.

Jordan

11 months ago by tim@bastelstu.be — view source

unread

Hi

That is an absurd example. Why would anyone use inheritance for that class
design? If what you are arguing is "if you look at use cases where
composition is clearly the correct choice then inheritance causes
problems", then I'm not sure what the point of the discussion is.

Okay, then please tell me in which use cases inheritance rather than
composition is the right choice? For the "Money" example, I've already
showcased how it would be broken and I can't think of any example where
inheritance would be superior to composition.

Yes, my example is dumb, but nevertheless it showcased that the native
and unchangeable operations would be unsound for child classes with
custom properties, because the operations would not be able to correctly
fill in the custom properties.

And even without custom properties, the behavior would be unsound
because the class name acts as an implicit unit:

 $twoMeters = new Distance(2);

 $fourSquareMeters = $twoMeters * $twoMeters;

Multiplying two distances does not result in a distance, but rather in
an area. Making it a Distance(4) would be incorrect.

So, which use case would be enabled by allowing inheritance without
being subtly broken by being unable to override the native operations?

As I've said yesterday in Stack Overflow chat: "Operator overloads
through a backdoor are even worse than actual operator overloads".

Best regards
Tim Düsterhus

11 months ago by Jordan LeDoux — view source

unread

The only solution I can think of at the moment is to impose the constraint
that when computing operator overloading, if the operands are both objects,
they must be of the exact same class.

When calculating using a method, it is clear which object should be
prioritized, so there is no risk of breaking down even if the method
accepts objects of different classes as arguments. It would be possible to
unify the behavior of the methods with operator overloading, but this would
be undesirable because the arguments would be contravariant.

This is something that there are established solutions for actually. It's
called Polymorphic Handler Resolution. This essentially means that when you
have two objects used with operators, if they share a parent class or a
class structure, the newest descendant is given priority. For example, if
you have class A as the parent, then class B and class C as children:

A + B = B, because it is a child class of A
B + A = B, because it is a child class of A
B + C = B, because neither are direct descendants
C + B = C, because neither are direct descendants

If we add the requirement that they must either be the same class as you
suggested, OR one must be a descendant of the other, then we eliminate the
B + C issue entirely, which makes sense for a BCMath object. How this would
actually be implemented in C is that both operands would be checked for
inheritance. If they are different classes and one is not the descendant of
the other, it would error. If they are different classes and one IS the
descendant of the other, then the handler for the descendant would be
called, and that handler would always return an instance of itself.

Jordan

11 months ago by tim@bastelstu.be — view source

unread

Hi

On 4/5/24 20:30, Jordan LeDoux wrote:> 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.

You are right of course. It should've read 10 USD.

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.

Opening up the class after-the-fact is always possible, should
reasonable use-cases be proposed that would be enabled by it. Closing it
down (i.e. marking it final), should it turn out that allowing to extend
it was a bad choice, is not possible. Thus 'final' is the safe default
choice.

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.

The operator overloads are already useless, because they can't usefully
return the subclassed type even of the currencies would match for the
money example.

 $fiveEuros = new Money(5, 'EUR');
 $tenEuros = new Money(10, 'EUR');

 $what = $fiveEuros + $tenEuros;

The only reasonable result for the implementation would be Number(15),
because the addition operator would not know what to do with the extra
fields.

Likewise:

 $fiveMeters = new Distance(500);
 $oneMinute = new Time(60000);

 $what = $fiveMeters * $oneMinute;

is equally unsound, because 5 meters per minute is something different
than 30000000, 300, 5, 0.08333333 or whatever the internal
representation of those classes looks like. Numbers with units are not
interchangeable with scalars.

Generically speaking getting back the Number class when invoking any of
the existing methods on an object of the child class drastically reduces
the little bit of usefulness a child class might have.

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.

It's a reasonably common source of bugs. Here's several closely-related
fixes:

https://github.com/php/php-src/commit/93becab506ac05a7bddce1ceaacb53d6657180ce
https://github.com/php/php-src/commit/a22558183309c05bb6bd7946ea1063143654f200
https://github.com/php/php-src/commit/85fbc6eaa6cd596f67d533091b50b2e2fcf9b601

I remember that other issues related to subclassing the date classes
have arisen in the past.

Best regards
Tim Düsterhus

11 months ago by Rowan Tommins [IMSoP] — view source

unread

A composed class
does not somehow prevent the accidental error of mixing currencies, it just
moves where that error would occur

It does not prevent someone accidentally attempting to mix currencies, but it absolutely prevents that mistake leading to bogus values in the application, because the methods available can all detect it and throw an Error.

If the class has a mixture of currency-aware methods, and methods / operator overloads inherited from Number, you can end up getting nonsensical results instead of an Error.

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 = $val1->getCurrency();
$val2Currency = $val2->getCurrency();
$val1 = $val1->convertToCommonCurrency();
$val2 = $val2->convertToCommonCurrency();
// perform the necessary calculations using operators
$answer = $answer->convertToCurrency($userCurrency);

You have missed out the key section: how do you actually add the two numbers? The addition MUST enforce the precondition that its operands are in the same currency; any other behaviour is nonsensical. So the definition of that method must be on the Money class, not inherited from Number.

If the add() method on Number is final, you'll need to define a new method $val1->addCurrency($val2). The existing add() method and operator overload will be inherited unchanged, but calling them won't just be useless, it will be dangerous, because they can give nonsensical results.

That's what makes composition the better design in this case, because the Money class can expose only the methods that actually have useful and safe behaviour.

The fact that the composed class can't add its own operator overloads is unfortunate; but allowing inheritance wouldn't solve that, because the inherited overloads are all wrong anyway.

Regards,
Rowan Tommins
[IMSoP]

11 months ago by Derick Rethans — view source

unread

$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.

I'm not sure I like this. Maybe we should be more strict here and
treat the $scale in constructor (and later withScale()) as the
actual scale for all operations.

For addition, it absolutely should expand scale like this, unless the
constructor also defines a default rounding type that is used in that
situation. All numbers, while arbitrary, will be finite, so addition
will always be exact and known based on inputs prior to calculation.

Treating scale like this isn't more strict, it's confusing. For
instance:
$numA = new Number('1.23', 2);
$numB = new Number('1.23456', 5);

$expandedScale1 = $numA + $numB; // 2.46456
$expandedScale2 = $numB + $numA; // 2.46456

$strictScale1 = $numA + $numB; // 2.46 assuming truncation
$strictScale2 = $numB + $numA; // 2.46456
I ran into this same issue with operand ordering when I was writing my
operator overload RFC.

There are ways you could do the overload implementation that would get
around this for object + object operations, but it's also
mathematically unsound and probably unexpected for anyone who is going
to the trouble of using an arbitrary precision library.

Addition and subtraction should automatically use the largest scale
from all operands. Division and multiplication should require a
specified scale.

I agree. I think add/subtract also should always take the largest scale
here.

cheers,
Derick

1 year ago by Derick Rethans — view source

unread

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):
https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#namespaces-in-extensions

The reason it should have "BC" is that it comes from "Basic
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.

That works fine as well. Especially as most people would add:

use BCMath\Number as Number;

and then just use "Number" everywhere.

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?

Feeling :-) Do we have precedence with other extension's objects
perhaps already?

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.

OK. That makes sense.

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.

Great.

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?

It's what we did for the Date extension, and the Random extension, but
in this case, it's probably not needed as you say.

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?

For that, yes. ValueErrors should be distinct from DivideByZeroError — I
think we do have both of those already:

php -r 'echo 8/0;'

Fatal error: Uncaught DivisionByZeroError: Division by zero in Command
line code on line 1

From the docs for ValueError:

"A ValueError is thrown when the type of an argument is correct but the
value of it is incorrect."

From the docs for DivisionByZeroError:

" DivisionByZeroError is thrown when an attempt is made to divide a
number by zero. "

Subclassing these for BCMath objects seems unnecessary therefore.

cheers,
Derick

--
https://derickrethans.nl | https://xdebug.org | https://dram.io

Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support

mastodon: @derickr@phpc.social @xdebug@phpc.social

1 year ago by Saki Takamachi — view source

unread

Hi Derick,

What is the main difference between getting a read-only property with
->value and getting the value using a method?

Feeling :-) Do we have precedence with other extension's objects
perhaps already?

It depends on the extension. Probably some readonly properties exist in mysqli for example. (However, those in mysqli are only readonly internally, and stubs are not readonly. Therefore, many IDEs do not display errors, so w e don't realize we've written incorrect code until runtime.)
The Readonly property was implemented since PHP 8.1, so inner classes implemented before then did not have the option of using the readonly property. As far as I know, no new inner classes with readonly properties or simple getters have been added since PHP 8.1. Therefore, there is currently no precedent that can be compared on the same terms as BCMath\Number….

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.

OK. That makes sense.

Note that we have removed $scale from the calculation method arguments, reflecting the discussion on the mailing list :)

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.

Great.

This has also been changed to reflect discussion, with slightly different results.

$num = new BcNum('1.23', 2);
$result = $num + '1.23456';
$result->value; // '2.46'
$result->scale; // 2

It's what we did for the Date extension, and the Random extension, but
in this case, it's probably not needed as you say.

For that, yes. ValueErrors should be distinct from DivideByZeroError — I
think we do have both of those already:

php -r 'echo 8/0;'

Fatal error: Uncaught DivisionByZeroError: Division by zero in Command
line code on line 1

From the docs for ValueError:

"A ValueError is thrown when the type of an argument is correct but the
value of it is incorrect."

From the docs for DivisionByZeroError:

" DivisionByZeroError is thrown when an attempt is made to divide a
number by zero. "

Subclassing these for BCMath objects seems unnecessary therefore.

Thanks. Sorry, I meant "If I create dedicated exception classes, do I need separate classes for each type of error?” I couldn't write it well in English.

Regards.

Saki

1 year ago by Aleksander Machniak — view source

unread

The Readonly property was implemented since PHP 8.1, so inner classes implemented before then did not have the option of using the readonly property. As far as I know, no new inner classes with readonly properties or simple getters have been added since PHP 8.1. Therefore, there is currently no precedent that can be compared on the same terms as BCMath\Number….

Enums have 'value' and 'name' properties. Although they are kinda special.

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

1 year ago by Saki Takamachi — view source

unread

Hi Aleksander,

Enums have 'value' and 'name' properties. Although they are kinda special.

I overlooked a few things.

https://www.php.net/manual/en/class.transliterator.php
https://www.php.net/manual/en/class.random-randomizer.php
https://www.php.net/manual/en/class.directory.php
https://www.php.net/manual/en/class.tidynode.php

These are classes that have properties marked with the readonly modifier within the current master branch. (Note, however, that I have not examined properties marked with @readonly)

There have been precedents, but they don't seem to be that many.

Regards.

Saki

11 months ago by tim@bastelstu.be — view source

unread

Hi

Thanks. Sorry, I meant "If I create dedicated exception classes, do I need separate classes for each type of error?” I couldn't write it well in English.

To answer this question, even if it is not necessary after all:

You should create separate classes for each type of error the user is
expected to handle differently so that developers to not need to check
the exception message to find out what the error was in their code.

Here's an insightful GitHub comment from Danack:
https://github.com/php/php-src/pull/9071#issuecomment-1193162754

And here's the PR adding a proper Exception hierarchy to ext/random:
https://github.com/php/php-src/pull/9220

For ext/date, here's the RFC adding the new Exception hierarchy:
https://wiki.php.net/rfc/datetime-exceptions. If I remember correctly,
the choices made for ext/date followed the precedent set by ext/random.

Best regards
Tim Düsterhus

1 year ago by Aleksander Machniak — view source

unread

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

Here's another question.

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

1 year ago by Saki Takamachi — view source

unread

Hi Aleksander,

Here's another question.

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

1 year ago by Aleksander Machniak — view source

unread

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.

If you write it as:

$result = $num->withScale(4)->add($num2);

it's not an extra line anymore. I also think that withScale() use will
be rare, as we have the scale in constructor.

I think the intention is more clear here, and I think it applies to all
cases you mentioned, including div or pow. If you know you need to
change the scale just add ->withScale(X) before.

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

1 year ago by Barney Laurance — view source

unread

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.

1 year ago by Saki Takamachi — view source

unread

Hi Barney,

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.

I was thinking about those features too. However, I'm concerned that proposing too many features will complicate the RFC and make it difficult to get it approved.

It might make sense to have a second vote on whether to implement those features.

Regards.

Saki

11 months ago by Barney Laurance — view source

unread

Hi again,

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.
I was thinking about those features too. However, I'm concerned that proposing too many features will complicate the RFC and make it difficult to get it approved.

Coming back to this point, I think these are basic features that people
would expect to be there - I think I would find just slightly
frustrating to start learning how to use a class like this and then
find that it doesn't have these functions. Casting and calling intval
or floatval all feel like slightly awkward workarounds that shouldn't
be needed in a greenfield project. We know that the string
inside the object is always a numeric string, so it should be easier to
parse it as an int than to parse it as a date or a JSON document. Code
doing the latter should stand out as odd looking.

11 months ago by Jordan LeDoux — view source

unread

On Thu, Apr 4, 2024 at 1:59 PM Barney Laurance barney@redmagic.org.uk
wrote:

Hi again,

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.

I was thinking about those features too. However, I'm concerned that proposing too many features will complicate the RFC and make it difficult to get it approved.

Coming back to this point, I think these are basic features that people
would expect to be there - I think I would find just slightly frustrating
to start learning how to use a class like this and then
find that it doesn't have these functions. Casting and calling intval or
floatval all feel like slightly awkward workarounds that shouldn't be
needed in a greenfield project. We know that the string
inside the object is always a numeric string, so it should be easier to
parse it as an int than to parse it as a date or a JSON document. Code
doing the latter should stand out as odd looking.

The class cannot guarantee that it can return a value in the type you
request however, so the way that is handled would need to be decided. The
value can easily be outside of the range of an int. Should it return a
float silently in that case for toInt()? What if the value is beyond the
range of a float? That would be a very rare situation, as floats can
represent extremely large numbers (with very reduced accuracy), but I would
expect it to throw an exception if that happened. Ideally an exception that
I could catch and ignore, since I can almost surely deal with that error in
most situations.

What about a number that is so small that it can't fit in a float? Similar
situation, though I expect it would occur slightly more often than a number
being too large to fit in a float, even though it would also be rare.

I think these helper functions belong in the RFC, but they aren't quite
straightforward, which is what I think Saki was alluding to.

Jordan

11 months ago by Barney Laurance — view source

unread

On Thu, Apr 4, 2024 at 1:59 PM Barney Laurance
barney@redmagic.org.uk wrote:
Hi again,
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.
I was thinking about those features too. However, I'm concerned that proposing too many features will complicate the RFC and make it difficult to get it approved.
Coming back to this point, I think these are basic features that
people would expect to be there - I think I would find just
slightly frustrating to start learning how to use a class like
this and then
find that it doesn't have these functions. Casting and calling
`intval` or `floatval` all feel like slightly awkward workarounds
that shouldn't be needed in a greenfield project. We know that the
string
inside the object is always a numeric string, so it should be
easier to parse it as an int than to parse it as a date or a JSON
document. Code doing the latter should stand out as odd looking.
The class cannot guarantee that it can return a value in the type you
request however, so the way that is handled would need to be decided.
The value can easily be outside of the range of an int. Should it
return a float silently in that case for toInt()? What if the value
is beyond the range of a float? That would be a very rare situation,
as floats can represent extremely large numbers (with very reduced
accuracy), but I would expect it to throw an exception if that
happened. Ideally an exception that I could catch and ignore, since I
can almost surely deal with that error in most situations.

What about a number that is so small that it can't fit in a float?
Similar situation, though I expect it would occur slightly more often
than a number being too large to fit in a float, even though it would
also be rare.

I think these helper functions belong in the RFC, but they aren't
quite straightforward, which is what I think Saki was alluding to.

Yes I agree there are subtleties to work out for these functions. I
don't think there's such a thing as a number to small to fit in a float.
Converting from decimal to float is always an approximation, sometimes
the best approximation available as a float will be either 0 or -0.

11 months ago by Saki Takamachi — view source

unread

Hi,

To be honest, I think it would be much easier to use if we could make it look like this:

Use precision instead of scale
The default maximum precision is 20 digits
The default rounding mode is HALF_UP
The constructor takes only $num arguments
The method can optionally specify any precision and any rounding mode when calculating
Operators always use only default values in their calculations

However, BCMath has worked with scale for a long time, so I'm not sure that introducing precision behavior here would be of any benefit to users...

Regards.

Saki

11 months ago by Saki Takamachi — view source

unread

Use precision instead of scale

The default maximum precision is 20 digits

The default rounding mode is HALF_UP

The constructor takes only $num arguments

The method can optionally specify any precision and any rounding mode when calculating

Operators always use only default values in their calculations

Ah, that takes away the benefit of arbitrary precision...
Maybe I'm sleepy, forget about this.

Regards.

Saki

11 months ago by Rowan Tommins [IMSoP] — view source

unread

https://wiki.php.net/rfc/support_object_type_in_bcmath

Based on the various discussions we've been having, I'd like to propose
a simplified handling of "scale".

I think there are two groups of users we are trying to help:

a) Users who want an "infinite" scale, and will round manually when
absolutely necessary, e.g. for display. The scale can't actually be
infinite in the case of calculations like 1/3, so they need some safe
cut-off.

b) Users who want to perform operations on a fixed scale, with
configurable rounding, e.g. for e-commerce pricing. They are not
interested in any larger scale, except possibly in some intermediate
calculations, when they want the same as group (a).

I propose:

The constructor accepts string|int $num only.
All operations accept an optional scale and rounding mode.
If no rounding mode is provided, the default behaviour is to truncate.
This means that (new BCMath\Number('20'))->div(3, 5) has the same result
as bcdiv('20', '3', 5) which is 6.66666
If a rounding mode is provided, the object transparently calculates
one extra digit of scale, then rounds according to the specified mode.
If no scale is provided, most operations will automatically calculate
the required scale, e.g. add will use the larger of the two scales. This
is the same as the current RFC.
If no scale is provided to div(), sqrt(), or pow(-$x), the result will
be calculated to the scale of the left-hand operand, plus 10. This is
the default behaviour in the current RFC.
Operator overloads behave the same as not specifying a scale or
rounding mode to the corresponding method. Therefore (new
BCMath\Number('20')) / (new BCMath\Number('3')) will result in
6.6666666666 - an automatic scale of 10, and truncation of further digits.

Compared to the current RFC, that means:

Remove the ability to customise "max expansion scale". For most users,
this is a technical detail which is more confusing than useful. Users in
group (b) will never encounter it, because they will specify scale
manually; advanced users in group (a) may want to customise the logic in
different ways anyway.
Remove the ability for a Number value to carry around its own default
rounding mode. Users in group (a) will never use it. Users in group (b)
are likely to want the same rounding in the whole application, but
providing it on every call to new Number() is no easier than providing
it on each fixed-scale calculation.
Remove the $maxExpansionScale and $roundingMode properties and
constructor parameters.
Remove withMaxExpansionScale and withRoundMode.
Remove all the logic around propagating rounding mode and expansion
scale between objects.

I've also noticed that the round method is currently defined as:

public function round(int $precision = 0, int $mode =
PHP_ROUND_HALF_UP): Number {}

Presumably $precision here is actually the desired scale of the result?
If so, it should probably be named $scale, as in the rest of the interface.

I realise it's called $precision in the global round() function; that's
presumably a mistake which is now hard to fix due to named parameters.

Ideally, it would be nice to have both roundToPrecision() and
roundToScale(), but as Jordan explained, an implementation which
actually calculated precision could be difficult and slow.

Regards,

--
Rowan Tommins
[IMSoP]

11 months ago by Saki Takamachi — view source

unread

Hi Rowan,
a).

I propose:

The constructor accepts string|int $num only.

All operations accept an optional scale and rounding mode.

If no rounding mode is provided, the default behaviour is to truncate. This means that (new BCMath\Number('20'))->div(3, 5) has the same result as bcdiv('20', '3', 5) which is 6.66666

If a rounding mode is provided, the object transparently calculates one extra digit of scale, then rounds according to the specified mode.

If no scale is provided, most operations will automatically calculate the required scale, e.g. add will use the larger of the two scales. This is the same as the current RFC.

If no scale is provided to div(), sqrt(), or pow(-$x), the result will be calculated to the scale of the left-hand operand, plus 10. This is the default behaviour in the current RFC.

Operator overloads behave the same as not specifying a scale or rounding mode to the corresponding method. Therefore (new BCMath\Number('20')) / (new BCMath\Number('3')) will result in 6.6666666666 - an automatic scale of 10, and truncation of further digits.

Compared to the current RFC, that means:

Remove the ability to customise "max expansion scale". For most users, this is a technical detail which is more confusing than useful. Users in group (b) will never encounter it, because they will specify scale manually; advanced users in group (a) may want to customise the logic in different ways anyway.

Remove the ability for a Number value to carry around its own default rounding mode. Users in group (a) will never use it. Users in group (b) are likely to want the same rounding in the whole application, but providing it on every call to new Number() is no easier than providing it on each fixed-scale calculation.

Remove the $maxExpansionScale and $roundingMode properties and constructor parameters.

Remove withMaxExpansionScale and withRoundMode.

Remove all the logic around propagating rounding mode and expansion scale between objects.

I have two questions.

The scale and rounding mode are not required for example in add, since the scale of the result will never be infinite and we can automatically calculate the scale needed to fit the result. Does adding those two options to all calculations mean adding them to calculations like add as well?
As Tim mentioned, it may be confusing to have an initial value separate from the mode of the round() method. Would it make sense to have an initial value of HALF_UP?

I've also noticed that the round method is currently defined as:

public function round(int $precision = 0, int $mode = PHP_ROUND_HALF_UP): Number {}

Presumably $precision here is actually the desired scale of the result? If so, it should probably be named $scale, as in the rest of the interface.

I realise it's called $precision in the global round() function; that's presumably a mistake which is now hard to fix due to named parameters.

Ideally, it would be nice to have both roundToPrecision() and roundToScale(), but as Jordan explained, an implementation which actually calculated precision could be difficult and slow.

There is good news about this. The RFC for bcround does not specify the argument names, and the implementer (me) has decided on the argument names.
And it's a change that hasn't even been merged into master yet, so I can change the argument name without any BC break.

Regards.

Saki

11 months ago by Saki Takamachi — view source

unread

postscript:

The precision of round() can be negative. I'm not sure if this should be called scale.

Regards.

Saki

11 months ago by Rowan Tommins [IMSoP] — view source

unread

The scale and rounding mode are not required for example in add, since the scale of the result will never be infinite and we can automatically calculate the scale needed to fit the result. Does adding those two options to all calculations mean adding them to calculations like add as well?

That's why I mentioned the two different groups of users. The scale and rounding mode aren't there for group (a), who just want the scale to be managed automatically; they are there for group (b), who want to guarantee a particular result has a particular scale. The result of $a->add($b, 2, Round::HALF_UP) will always be the same as $a->add($b)->round(Round::HALF_UP) but is more convenient, and in some cases more efficient, since it doesn't calculate unnecessary digits.

Remember also the title and original aim of the RFC: add object support to BCMath. The scale parameter is already there on the existing functions (bcadd, bcmul, etc), so removing it on the object version would be surprising. The rounding mode is a new feature, but there doesn't seem a good reason not to include it everywhere as well.

As Tim mentioned, it may be confusing to have an initial value separate from the mode of the round() method. Would it make sense to have an initial value of HALF_UP?

Again, the aim was to match the functionality of the existing functions. It's likely that users will migrate code written using bcdiv() to use BCMath\Number->div() and expect it to work the same, at least when specifying a scale. Having it behave differently by rounding up the last digit by default seems like a bad idea.

Thinking about the implementation, the truncation behaviour also makes sense: the library isn't actually rounding anything, it's calculating digit by digit, and stopping when it reaches the requested scale.

The whole concept of rounding is something that we are adding, presumably by passing $scale+1 to the underlying library functions. It's a nice feature to add, but not one that should be on by default, given we're not writing the extension from scratch.

Regards,
Rowan Tommins
[IMSoP]

11 months ago by Saki Takamachi — view source

unread

Hi Rowan,

That's why I mentioned the two different groups of users. The scale and rounding mode aren't there for group (a), who just want the scale to be managed automatically; they are there for group (b), who want to guarantee a particular result has a particular scale. The result of $a->add($b, 2, Round::HALF_UP) will always be the same as $a->add($b)->round(Round::HALF_UP) but is more convenient, and in some cases more efficient, since it doesn't calculate unnecessary digits.

Remember also the title and original aim of the RFC: add object support to BCMath. The scale parameter is already there on the existing functions (bcadd, bcmul, etc), so removing it on the object version would be surprising. The rounding mode is a new feature, but there doesn't seem a good reason not to include it everywhere as well.

Ah, I understand. There may certainly be use cases where demand specifies a scale smaller than the maximum scale in order to save computational costs. That makes sense to me.

Also, considering the two groups you presented, the group that wants to control the scale is probably not going to use operator calculations, even though they could provide various options in the constructor. Unless look at the contents of objects in the calculation process using var_dump or something, can't understand the state just by looking at the code, which can lead to bugs. They probably don't like that kind of code.

Again, the aim was to match the functionality of the existing functions. It's likely that users will migrate code written using bcdiv() to use BCMath\Number->div() and expect it to work the same, at least when specifying a scale. Having it behave differently by rounding up the last digit by default seems like a bad idea.

Thinking about the implementation, the truncation behaviour also makes sense: the library isn't actually rounding anything, it's calculating digit by digit, and stopping when it reaches the requested scale.

The whole concept of rounding is something that we are adding, presumably by passing $scale+1 to the underlying library functions. It's a nice feature to add, but not one that should be on by default, given we're not writing the extension from scratch.

I was thinking about this today, and I think both are correct opinions on whether to set the initial value to HALF_UP or TOWARD_ZERO. It's just a matter of prioritizing whether consistency with existing behavior or consistency within a class, and they can never be met simultaneously.

Therefore, I am considering adding a more detailed explanation to the RFC on this issue and taking a second vote to decide.

Regards.

Saki

11 months ago by Rowan Tommins [IMSoP] — view source

unread

I was thinking about this today, and I think both are correct opinions on whether to set the initial value to HALF_UP or TOWARD_ZERO. It's just a matter of prioritizing whether consistency with existing behavior or consistency within a class, and they can never be met simultaneously.

Yes, I agree there's a dilemma there.

The extra point in favour of TOWARD_ZERO is that it's more efficient, because we don't have to over-calculate and round, just pass scale directly to the implementation. Any other option makes for unnecessary extra calculation in code like this:

$total = new Number('20');
$raw_frac = $total / 7;
$rounded_frac = $raw_frac->round(2, Round::HALF_UP);

If HALF_UP rounding is the implied default, we have to calculate with scale 11 giving 1.42857142857, round to 1.4285714286, then round again to 1.43.

If truncation / TOWARD_ZERO is the implied default, we only calculate with scale 10 giving 1.4285714285 and then round once to 1.43.

(Of course, in this example, the most efficient would be for the user to write $rounded_frac = $total->div(7, 2, Round::HALF_UP) but they might have reasons to keep the division and rounding separate.)

Regards,

Rowan Tommins
[IMSoP]

11 months ago by Saki Takamachi — view source

unread

Hi Rowan,

Yes, I agree there's a dilemma there.

The extra point in favour of TOWARD_ZERO is that it's more efficient, because we don't have to over-calculate and round, just pass scale directly to the implementation. Any other option makes for unnecessary extra calculation in code like this:

$total = new Number('20');
$raw_frac = $total / 7;
$rounded_frac = $raw_frac->round(2, Round::HALF_UP);

If HALF_UP rounding is the implied default, we have to calculate with scale 11 giving 1.42857142857, round to 1.4285714286, then round again to 1.43.

If truncation / TOWARD_ZERO is the implied default, we only calculate with scale 10 giving 1.4285714285 and then round once to 1.43.

(Of course, in this example, the most efficient would be for the user to write $rounded_frac = $total->div(7, 2, Round::HALF_UP) but they might have reasons to keep the division and rounding separate.)

Thanks, when I expand on this issue, I'll also mention the pros and cons of both, including the points you mentioned.

Regards,

Saki

11 months ago by Saki Takamachi — view source

unread

Hi internals,

I have reflected the discussion up to this point in the RFC.
https://wiki.php.net/rfc/support_object_type_in_bcmath

However, the point that the argument of round() is "precision" has not yet been reflected in the RFC. The argument names of standard's round() are also the same, and we are currently discussing this point in the implementation PR of bcround().

Regards,

Saki

11 months ago by Saki Takamachi — view source

unread

Hi,

If there is no further discussion, I will start voting tomorrow. (I haven't decided on the time yet.)
https://wiki.php.net/rfc/support_object_type_in_bcmath

Regards,

Saki

11 months ago by drealecs@gmail.com — view source

unread

Hi,

If there is no further discussion, I will start voting tomorrow. (I
haven't decided on the time yet.)
https://wiki.php.net/rfc/support_object_type_in_bcmath

Just one small note from me, for mod operation, related to scale there is a
mention of "Use the scale of the dividend as is".
In reality, I think it should be the same as add and sub, "The larger scale
of the two values is applied".
In this way, something like this can work by default: https://3v4l.org/NismE

Regards,
Alex

11 months ago by Saki Takamachi — view source

unread

Hi Alex,

Just one small note from me, for mod operation, related to scale there is a mention of "Use the scale of the dividend as is".
In reality, I think it should be the same as add and sub, "The larger scale of the two values is applied".
In this way, something like this can work by default: https://3v4l.org/NismE

Thanks for the good pointers! This was an oversight on my part. I will update the RFC.

Regards,

Saki

[RFC] [Discussion] Support object type in BCMath

-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net]

-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net]

-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net]

-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net]

-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net]

-- Aleksander Machniak Kolab Groupware Developer [https://kolab.org] Roundcube Webmail Developer [https://roundcube.net]

Regards,

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]

--
Aleksander Machniak
Kolab Groupware Developer [https://kolab.org]
Roundcube Webmail Developer [https://roundcube.net]