Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-30 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-30 Thread Alexandru Pătrănescu
On Tue, Apr 30, 2024 at 7:31 AM Saki Takamachi  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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-29 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-14 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-10 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-10 Thread Rowan Tommins [IMSoP]
On 10 April 2024 10:38:44 BST, Saki Takamachi  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]


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-10 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-10 Thread Rowan Tommins [IMSoP]



On 10 April 2024 00:36:21 BST, Saki Takamachi  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]


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Saki Takamachi
postscript:

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

Regards.

Saki

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Saki Takamachi
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.6
> 
> - 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.66 - 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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Rowan Tommins [IMSoP]

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


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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Tim Düsterhus

Hi

On 4/8/24 02:08, Saki Takamachi wrote:

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


Agree. I describe it explicitly during implementation.



I don't see a change made, so perhaps I was unclear in what I said. What 
I meant was: Please add a full stub example including all the 
interfaces, properties, and methods. It's currently split across 
multiple code blocks and the interfaces are missing entirely.


Example:

final readonly class Number implements \Stringable {
public string $value;
// Further properties omitted for brevity.

public function add(Number|string|int $num): Number {}
// Further methods omitted for brevity.
}


- I don't want to expand the scope of your RFC further than necessary, but for the 
rounding mode, I wonder if we should first add the RoundingMode enum that has been 
suggested during the discussion of the "Add 4 new rounding modes to round() 
function" RFC. It would make the type for the `Number::$roundMode` property much 
clearer than the current `int`. I expect the addition of such an enum to be a pretty 
simple RFC that would not need much discussion. I'd be happy to co-author it.


Oh, that's a good idea. Makes sense. I think it would be simpler to prepare an 
RFC separate from this RFC, so I'm going to create one right away. Once you 
have a certain amount of content, I would be happy if you could check it out 
and make corrections if necessary.



Sure, just send me an email and I'm happy to look over it before you put 
it up for discussion.



- For `format()`, I'm not sure whether we should actually add the function. 
While we have number_format() for regular numbers with the same signature, it 
fails to account for the different language and cultures in the world. 
Specifically `number_format()` cannot correctly format numbers for en_IN (i.e. 
English in India). In India numbers are not separated every three digits, but 
rather the three right-most digits and then every two digits. Here's in 
example: 1,23,45,67,890. I believe formatting should be best left for ext/intl.


I'm not very familiar with ext/intl, but is there a way to format a string type 
number without converting it to a float?


It appears the underlying icu4c library supports formatting numeric 
strings, yes:


https://unicode-org.github.io/icu-docs/apidoc/dev/icu4c/classicu_1_1NumberFormat.html#a8ed2ca7b9a65bf08c4ef81bbf9680f0d


- I'm not sure if the priority for the rounding modes is sound. My gut feeling 
is that operations on numbers with different rounding modes should be 
disallowed or made explicit during the operation (much like the scale for a 
division), but I'm not an expert in designing numeric APIs, so I might be wrong 
here.


As mentioned in the RFC, the problem here is commutative calculations (add, 
sub, mul). This means that even if specify `$roundingMode` during these 
calculations, `$roundingMode` will not be used in these calculations. 
Calculations that use `$roundingMode` are divs, etc. If allow `$roundingMode` 
to be specified with add, intuitively it feels like the result of add will be 
rounded.

Also, it is not possible to specify `$roundMode` in calculations using 
operators.

However, if the user calculates two numbers with different rounding modes, and 
it is intentional rather than a mistake, I can't imagine what kind of result 
the user would want to get. Therefore, it may be better to make this an error.

Is it appropriate to throw a value error in that case?


I think making this an error would be appropriate. Generally speaking: 
Removing errors later is always possible if we find out that an 
operation is safe and well-defined. Adding an error if we find out that 
an operation is unsafe will result in breaking changes, thus we should 
get it right on the first try.


Best regards
Tim Düsterhus


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Saki Takamachi
> - 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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-09 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-07 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-07 Thread Saki Takamachi
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-07 Thread Rowan Tommins [IMSoP]

On 07/04/2024 18:09, Tim Düsterhus wrote:
- 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]


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-07 Thread Tim Düsterhus

Hi

On 4/6/24 17:07, Saki Takamachi wrote:

To you all:
The discussions thus far have been reflected in the RFC. I would be very happy if you 
could check it out. (Thanks Tim, the  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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-07 Thread Tim Düsterhus

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-06 Thread Jordan LeDoux
On Sat, Apr 6, 2024 at 6:45 AM Rowan Tommins [IMSoP] 
wrote:

> On 06/04/2024 07:24, Saki Takamachi 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-06 Thread Saki Takamachi
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  tag is very easy to read.)
https://wiki.php.net/rfc/support_object_type_in_bcmath

Regards.

Saki

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-06 Thread Rowan Tommins [IMSoP]

On 06/04/2024 07:24, Saki Takamachi 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.


Regards,

--
Rowan Tommins
[IMSoP]


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-06 Thread Tim Düsterhus

Hi

On 4/5/24 22:04, Jordan LeDoux wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-06 Thread Rowan Tommins [IMSoP]



On 5 April 2024 19:30:24 BST, Jordan LeDoux  wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-06 Thread Saki Takamachi
> 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?

1. User freedom (inheritability)
2. Commutativity of computation
3. 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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Saki Takamachi
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.

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Saki Takamachi
Hi Tim, Jordan,On Fri, Apr 5, 2024 at 1:00 PM Tim Düsterhus  wrote:Hi

On 4/5/24 21:42, Saki Takamachi wrote:
> 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üsterhusThat 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 
One of the points Tim makes that actually matters is how to get user-defined properties on child classes to be carried over to new instances when operators are used to calculate them. (It doesn't really matter that the property is private.)For example, suppose you define a tax rate in a child class.My country currently has a mix of 10% and 8% tax rates, so the tax rate property could contain either 8 or 10.The question here is, if add objects with different tax rates, what should the tax rate of the resulting object be? It may be that we shouldn't calculate objects with different tax rates in the first place, but we need to clarify the behavior when we do actually do so.I've come up with a few options, but none of them are particularly smart. In fact, it's a very bad idea...- If a child class has its own properties, their values ​​must match exactly before any calculations can be performed using operators.- Properties defined in child classes are always uninitialized when calculated by operators.- Always apply the state of the left operandAlso, as I was thinking about this, I realized that $roundMode has the same problem. When calculating with an operator, I needed to consider what should happen if two objects have different $roundModes.I'll try to think of some more good ideas, but if you have any suggestions, please let me know.Regards.Saki

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Jordan LeDoux
On Fri, Apr 5, 2024 at 1:00 PM Tim Düsterhus  wrote:

> Hi
>
> On 4/5/24 21:42, Saki Takamachi wrote:
> > 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Jordan LeDoux
On Fri, Apr 5, 2024 at 12:42 PM Saki Takamachi  wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Tim Düsterhus

Hi

On 4/5/24 21:42, Saki Takamachi wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Tim Düsterhus

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(6);

$what = $fiveMeters * $oneMinute;

is equally unsound, because 5 meters per minute is something different 
than 3000, 300, 5, 0.0833 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Saki Takamachi
Hi,On Fri, Apr 5, 2024 at 2:48 AM Tim Düsterhus  wrote: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
I've thought twice about making classes final.The return type issue can be solved by specifying static, but as Barney mentioned, there is a problem with operator overloading, for example addition, where the 

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Jordan LeDoux
On Fri, Apr 5, 2024 at 2:48 AM Tim Düsterhus  wrote:

> 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Barney Laurance


On 05/04/2024 09:37, Saki Takamachi wrote:

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.


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Barney Laurance



On 05/04/2024 10:48, Tim Düsterhus wrote:


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.


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Tim Düsterhus

Hi

On 4/5/24 02:20, Saki Takamachi wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Tim Düsterhus

Hi

On 4/4/24 02:54, Jordan LeDoux wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Saki Takamachi
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  instead of  the code 
> examples will get syntax highlighting, making them more readable.

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

Regards.

Saki

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Tim Düsterhus

Hi

[cleaning out CC list]

On 4/4/24 08:29, Saki Takamachi wrote:

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  instead of  the 
code examples will get syntax highlighting, making them more readable.


Best regards
Tim Düsterhus


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-05 Thread Barney Laurance


On 05/04/2024 01:20, Saki Takamachi 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 - 
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)


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Saki Takamachi
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.0001`, 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 
>> 

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Jordan LeDoux
On Thu, Apr 4, 2024 at 2:19 PM Barney Laurance 
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Barney Laurance


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



On Thu, Apr 4, 2024 at 1:59 PM Barney Laurance 
 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.

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Barney Laurance

On 04/04/2024 01:54, Jordan LeDoux wrote:



On Tue, Apr 2, 2024 at 5:43 PM Saki Takamachi  wrote:


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.




Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Jordan LeDoux
On Thu, Apr 4, 2024 at 1:59 PM Barney Laurance 
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Barney Laurance

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.




Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Christian Schneider
Am 04.04.2024 um 08:29 schrieb Saki Takamachi :
> 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-04 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Jordan LeDoux
On Wed, Apr 3, 2024 at 7:31 PM Saki Takamachi  wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Saki Takamachi
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Jordan LeDoux
On Tue, Apr 2, 2024 at 5:43 PM Saki Takamachi  wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-03 Thread Barney Laurance

On 03/04/2024 01:42, Saki Takamachi wrote:

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Saki Takamachi
PS: Yep, this is pretty much Jordan's library idea.

Saki


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Jordan LeDoux
On Tue, Apr 2, 2024 at 5:05 PM Jordan LeDoux 
wrote:

>
>
> On Tue, Apr 2, 2024 at 4:50 PM Saki Takamachi  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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Jordan LeDoux
On Tue, Apr 2, 2024 at 4:50 PM Saki Takamachi  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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Saki Takamachi
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:
> 
> 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 two use cases at issue here are when the div and pow's exponent are 
negative values. 

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Jordan LeDoux
On Tue, Apr 2, 2024 at 10:24 AM Jordan LeDoux 
wrote:

>
>
> On Tue, Apr 2, 2024 at 3:12 AM Lynn  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. * 10.0` 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, 

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Jordan LeDoux
On Tue, Apr 2, 2024 at 3:12 AM Lynn  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. * 10.0` 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Barney Laurance

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.


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Derick Rethans
On Fri, 29 Mar 2024, Jordan LeDoux wrote:

> On Wed, Mar 27, 2024 at 12:08 AM Aleksander Machniak  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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Saki Takamachi
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. * 10.0` 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.33.
```

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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Lynn
On Tue, Apr 2, 2024 at 11:17 AM Jordan LeDoux 
wrote:

>
>
> On Sat, Mar 30, 2024 at 5:09 PM Saki Takamachi  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.3'
>>
>> $div = $num->div($num2, 20);
>> $div->getNumber(); // '0.333'
>> $div->getNumber(7); // ‘0.333'
>>
>>
>> // case 2
>> $num = new Number('1.11');
>> $num2 = new Number('3');
>>
>> $div = $num->div($num2, 3);
>> $div->getNumber(); // '0.370'
>> $div->getNumber(7); // ‘0.370'
>> ```
>>
>> 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. * 10.0` 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"?


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-04-02 Thread Jordan LeDoux
On Sat, Mar 30, 2024 at 5:09 PM Saki Takamachi  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.3'
>
> $div = $num->div($num2, 20);
> $div->getNumber(); // '0.333'
> $div->getNumber(7); // ‘0.333'
>
>
> // case 2
> $num = new Number('1.11');
> $num2 = new Number('3');
>
> $div = $num->div($num2, 3);
> $div->getNumber(); // '0.370'
> $div->getNumber(7); // ‘0.370'
> ```
>
> 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-30 Thread Saki Takamachi
Another div case:
```

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

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

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

Regards.

Saki


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-30 Thread Saki Takamachi
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.3'

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


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

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

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

Regards.

Saki

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-29 Thread Jordan LeDoux
On Wed, Mar 27, 2024 at 12:08 AM Aleksander Machniak  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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-28 Thread Saki Takamachi
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-28 Thread Aleksander Machniak

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-28 Thread Saki Takamachi
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?):
>>> 
>>> >> $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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-28 Thread Derick Rethans
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?):
> > 
> >  > $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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-27 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-27 Thread Aleksander Machniak

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-27 Thread Aleksander Machniak

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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.3`, 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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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?):
> 
>  $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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Barney Laurance

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.


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Aleksander Machniak

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Derick Rethans
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?):

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
Hi,

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

Regards.

Saki


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Larry Garfield
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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Aleksander Machniak

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_enroll=true=%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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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_enroll=true=%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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Aleksander Machniak

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Barney Laurance

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.


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Saki Takamachi
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

Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-26 Thread Barney Laurance

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.


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-25 Thread Barney Laurance

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


Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-25 Thread Barney Laurance

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?


[PHP-DEV] [RFC] [Discussion] Support object type in BCMath

2024-03-24 Thread Saki Takamachi
Hi internals,

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

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

Regards.

Saki


  1   2   >