On 10 April 2024 00:36:21 BST, Saki Takamachi <s...@sakiot.com> wrote:
>- The scale and rounding mode are not required for example in add, since the
>scale of the result will never be infinite and we can automatically calculate
>the scale needed to fit the result. Does adding those two options to all
>calculations mean adding them to calculations like add as well?
That's why I mentioned the two different groups of users. The scale and
rounding mode aren't there for group (a), who just want the scale to be managed
automatically; they are there for group (b), who want to guarantee a particular
result has a particular scale. The result of $a->add($b, 2, Round::HALF_UP)
will always be the same as $a->add($b)->round(Round::HALF_UP) but is more
convenient, and in some cases more efficient, since it doesn't calculate
unnecessary digits.
Remember also the title and original aim of the RFC: add object support to
BCMath. The scale parameter is already there on the existing functions (bcadd,
bcmul, etc), so removing it on the object version would be surprising. The
rounding mode is a new feature, but there doesn't seem a good reason not to
include it everywhere as well.
>- As Tim mentioned, it may be confusing to have an initial value separate from
>the mode of the `round()` method. Would it make sense to have an initial value
>of HALF_UP?
Again, the aim was to match the functionality of the existing functions. It's
likely that users will migrate code written using bcdiv() to use
BCMath\Number->div() and expect it to work the same, at least when specifying a
scale. Having it behave differently by rounding up the last digit by default
seems like a bad idea.
Thinking about the implementation, the truncation behaviour also makes sense:
the library isn't actually rounding anything, it's calculating digit by digit,
and stopping when it reaches the requested scale.
The whole concept of rounding is something that we are adding, presumably by
passing $scale+1 to the underlying library functions. It's a nice feature to
add, but not one that should be on by default, given we're not writing the
extension from scratch.
Regards,
Rowan Tommins
[IMSoP]