[RFC] [Discussion] Support object type in BCMath

Barney Laurance

2 years ago

On 24/03/2024 13:13, Saki Takamachi wrote:

> I want to start the discussion on the PHP RFC: Support object type in BCMath.

I suggest renaming `setScale` to `withScale`. Although the docs will
make clear that the object is immutable, `set` is associated with
mutation and might be confusing. `with` is not as well known as a prefix
but is associated with immutable objects. Also as with the value, any
reason not to make the scale a pubic readonly property?

Barney Laurance

2 years ago

On 24/03/2024 13:13, Saki Takamachi wrote:

> I want to start the discussion on the PHP RFC: Support object type in BCMath.

I work on OLTP application using BCMath for money, and I would like to
refactor to value objects (although we could also convert to using
ints), so this is very relevant to me. Liking the RFC a lot. Is there
any reason not to give the BcNum class a public readonly string `value`
property? Would just save a few characters of typing to use value
instead of getValue().

Barney Laurance

2 years ago

On 24/03/2024 13:13, Saki Takamachi wrote:

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

Saki Takamachi

2 years ago

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

Barney Laurance

2 years ago

On 2024-03-26 11:35, Saki Takamachi wrote:

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

Saki Takamachi

2 years ago

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

Larry Garfield

2 years ago

On Tue, Mar 26, 2024, at 12:50 PM, Saki Takamachi wrote:

> Hi Barney,
>
>> No, that's not quite what I meant - I meant more like the opposite:
>>
>>
>> ```
>> $bcNum = new BcNum('1234567890123456789.23456789');
>> echo $bcNum->format(8, '.', ',') // 1,234,567,890,123,456,789.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

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

Hi,

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

Regards.

Saki

A.L.E.C

2 years ago

On 24.03.2024 14:13, Saki Takamachi wrote:

> Hi internals,
>
> I want to start the discussion on the PHP RFC: Support object type in BCMath.
>
> 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

Saki Takamachi

2 years ago

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

A.L.E.C

2 years ago

On 26.03.2024 14:35, Saki Takamachi wrote:

> Hi Aleksander,
>
>> Was BCMath\Number considered instead of BcNum?
>
> Yes, that was one of the candidates. However, as far as I know, there are no examples of PHP internal classes having namespaces.
> Also, if use a namespace, the code will be written as `new Number()`, which is likely to conflict with existing code. In fact, if take a look at GitHub Code Search, you'll find 3.2k results.
> 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

Saki Takamachi

2 years ago

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

Derick Rethans

2 years ago

On Sun, 24 Mar 2024, Saki Takamachi wrote:

> Hi internals,
>
> I want to start the discussion on the PHP RFC: Support object type in BCMath.
>
> 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

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

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

A.L.E.C

2 years ago

On 27.03.2024 01:03, Saki Takamachi wrote:

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

Saki Takamachi

2 years ago

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

Jordan LeDoux

2 years ago

On Wed, Mar 27, 2024 at 12:08 AM Aleksander Machniak <alec@alec.pl> wrote:

> On 27.03.2024 01:03, Saki Takamachi wrote:
> >> $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

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

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

Jordan LeDoux

2 years ago

On Sat, Mar 30, 2024 at 5:09 PM Saki Takamachi <saki@sakiot.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

Lynn

2 years ago

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

>
>
> On Sat, Mar 30, 2024 at 5:09 PM Saki Takamachi <saki@sakiot.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"?

Saki Takamachi

2 years ago

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

Barney Laurance

2 years ago

On 2024-04-02 12:26, Saki Takamachi wrote:

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

Jordan LeDoux

2 years ago

On Tue, Apr 2, 2024 at 3:12 AM Lynn <kjarli@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:

1. Each number constructor allowed for an optional input scale.
2. 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.
3. If the input scale was provided, the determined scale is set to that
value.
4. Otherwise, the determined scale at construction is set to 10 or the
implicit scale of "number of digits", whichever is larger.
5. 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.
6. 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.
7. 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

Jordan LeDoux

2 years ago

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

>
>
> On Tue, Apr 2, 2024 at 3:12 AM Lynn <kjarli@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:
>
> 1. Each number constructor allowed for an optional input scale.
> 2. 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.
> 3. If the input scale was provided, the determined scale is set to that
> value.
> 4. Otherwise, the determined scale at construction is set to 10 or the
> implicit scale of "number of digits", whichever is larger.
> 5. 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.
> 6. 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.
> 7. 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

Derick Rethans

2 years ago

On Fri, 29 Mar 2024, Jordan LeDoux wrote:

> On Wed, Mar 27, 2024 at 12:08 AM Aleksander Machniak <alec@alec.pl> wrote:
>
> > On 27.03.2024 01:03, Saki Takamachi wrote:
> > >> $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

Derick Rethans

2 years ago

On Wed, 27 Mar 2024, Saki Takamachi wrote:

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

Saki Takamachi

2 years ago

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

A.L.E.C

2 years ago

On 28.03.2024 14:30, Saki Takamachi wrote:

> 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

Saki Takamachi

2 years ago

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

Tim Düsterhus

2 years ago

Hi

On 3/28/24 14:30, Saki Takamachi wrote:

> 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

A.L.E.C

2 years ago

On 24.03.2024 14:13, Saki Takamachi wrote:

> Hi internals,
>
> I want to start the discussion on the PHP RFC: Support object type in BCMath.
>
> https://wiki.php.net/rfc/support_object_type_in_bcmath

Here's another question.

1. Since we have withScale(), do we need to inherit the $scale argument
from the functional API? Can't we derive it from the object the method
is being invoked on?

So, instead, e.g.

public function add(BcNum|string|int $num, ?int $scale = null): BcNum {}
public function sqrt(?int $scale = null): BcNum {}

I'd suggest:

public function add(BcNum|string|int $num): BcNum {}
public function sqrt(): BcNum {}

but I have no clue about BCMath.

--
Aleksander Machniak
Kolab Groupware Developer        [https://kolab.org]
Roundcube Webmail Developer  [https://roundcube.net]
----------------------------------------------------
PGP: 19359DC1 # Blog: https://kolabian.wordpress.com

Saki Takamachi

2 years ago

Hi Aleksander,

> Here's another question.
>
> 1. Since we have withScale(), do we need to inherit the $scale argument from the functional API? Can't we derive it from the object the method is being invoked on?
>
> So, instead, e.g.
>
> public function add(BcNum|string|int $num, ?int $scale = null): BcNum {}
> public function sqrt(?int $scale = null): BcNum {}
>
> I'd suggest:
>
> public function add(BcNum|string|int $num): BcNum {}
> public function sqrt(): BcNum {}
>
> but I have no clue about BCMath.

Yeah, you're right. By using `withScale` before calculating, we can obtain results of arbitrary precision without using Scale during calculation.

The code in that case would look like this:
```
$num = new Number('1.23');
$num2 = new Number('4.56');

$numRescaled = $num->withScale(4);

$result = $numRescaled->add($num2);
$result->value; // '5.7900'
$result->scale; // 4
```

On the other hand, if  allow the calculation method to specify a scale, we can write it like this:
```
$num = new Number('1.23');
$num2 = new Number('4.56');

$result = $num->add($num2, 4);
$result->value; // '5.7900'
$result->scale; // 4
```

It's just one less line, but that's the only reason to support the `$scale` argument.

However, for calculations other than mul and div, the calculation results will always fit within the scale.
```
$num = new Number('1.23'); // scale 2
$num2 = new Number('1'); // scale 0

$num->add($num2); // '2.23', scale 2 is enough
$num->sub($num2); // '0.23', scale 2 is enough
$num->mod($num2); // '0.23', scale 2 is enough
```

On the other hand, for mul, div, and pow, the original `$num` scale may not be enough.
```
$num = new Number('1.23'); // scale 2
$num2 = new Number('1.1'); // scale 1

$num->mul($num2); // '1.353' be '1.35', scale 2 is not enough
```

```
$num = new Number('1.23'); // scale 2
$num2 = new Number('4'); // scale 0

$num->div($num2); // '0.3075' be '0.30', scale 2 is not enough
$num->pow($num2); // '2.28886641', be '2.28' scale 2 is not enough
```

For mul and pow, can calculate the required scale. However, for div, the calculation may never end, such as `0.33333....`, so it is not possible to calculate the scale to fit the result completely.

In cases like this, we thought that some users would want to easily specify the scale, so we created an easy-to-use signature.

However, it may not be necessary to specify scale for all calculation functions. Is it reasonable to specify only mul, div, pow, or only div?

Regards.

Saki

A.L.E.C

2 years ago

On 27.03.2024 01:36, Saki Takamachi wrote:

> 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

Barney Laurance

2 years ago

On 2024-03-24 13:13, Saki Takamachi wrote:

> I want to start the discussion on the PHP RFC: Support object type in
> BCMath.

Do we also need `toFloat` and `toInt` functions? Seems like using
explicit functions will be safer than casting.

For toInt I'd expect an exception if the value is outside the range of
possible ints. For toFloat it might be nice to have a flag
argument to give the developer the choice of having it throw if the
value is outside the range of floats or return INF or -INF,
or possibly the user should just check for infinite values themselves.

Saki Takamachi

2 years ago

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

Barney Laurance

2 years ago

Hi again,

On 27/03/2024 00:40, Saki Takamachi wrote:

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

Jordan LeDoux

2 years ago

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

> Hi again,
>
> On 27/03/2024 00:40, Saki Takamachi wrote:
>
> 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

Barney Laurance

2 years ago

On 04/04/2024 22:10, Jordan LeDoux wrote:

>
>
> On Thu, Apr 4, 2024 at 1:59 PM Barney Laurance
> <barney@redmagic.org.uk> wrote:
>
>     Hi again,
>
>     On 27/03/2024 00:40, Saki Takamachi wrote:
>>>     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.

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

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

Rowan Tommins [IMSoP]

2 years ago

On 24/03/2024 13:13, Saki Takamachi wrote:

> 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]

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

postscript:

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

Regards.

Saki

Rowan Tommins [IMSoP]

2 years ago

On 10 April 2024 00:36:21 BST, Saki Takamachi <saki@sakiot.com> wrote:

>- 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]

Saki Takamachi

2 years ago

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

Rowan Tommins [IMSoP]

2 years ago

On 10 April 2024 10:38:44 BST, Saki Takamachi <saki@sakiot.com> wrote:

>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]

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

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

Saki Takamachi

2 years ago

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

Alexandru Pătrănescu

2 years ago

On Tue, Apr 30, 2024 at 7:31 AM Saki Takamachi <saki@sakiot.com> wrote:

> 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

Saki Takamachi

2 years ago

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