Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath
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
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
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
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
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
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
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
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
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
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
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
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
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
> - 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
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
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
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
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
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
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
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
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
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
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
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
> 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
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
Hi Tim, Jordan,On Fri, Apr 5, 2024 at 1:00 PM Tim Düsterhuswrote: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
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
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
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
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
Hi,On Fri, Apr 5, 2024 at 2:48 AM Tim Düsterhuswrote: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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
PS: Yep, this is pretty much Jordan's library idea. Saki
Re: [PHP-DEV] [RFC] [Discussion] Support object type in BCMath
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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