On Wed, 27 Mar 2024, Saki Takamachi wrote:
> > - You've picked as class name "BcNum". Following
> > our naming guidelines, that probably should be \BCMath\Num (or
> > \BC\Num, but that is less descriptive):
> >
> > https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#namespaces-in-extensions
> >
> > The reason it *should* have "BC" is that it comes from "Basic
> > Calculator" (https://www.php.net/manual/en/book.bc.php#118203)
>
>
> I re-read the namespace RFC again. I also re-read the RFC regarding
> class naming conventions.
> https://wiki.php.net/rfc/namespaces_in_bundled_extensions
> https://wiki.php.net/rfc/class-naming
>
> There's no need for the namespace to follow class naming conventions,
> but the acronym doesn't seem to need to be pascal-case anyway (I
> remembered it incorrectly). However, the RFC states that the
> extension's namespace must match the extension name, so it seems
> correct in this case for the namespace to be `BCMath`.
>
> And indeed, looking at the example in the namespace RFC,
> `BCMath\Number` might be appropriate in this case (I think I was
> sleepy yesterday).
>
> I changed `BcNum` to `BCMath\Number` in my RFC.
That works fine as well. Especially as most people would add:
use BCMath\Number as Number;
and then just use "Number" everywhere.
> > - Should ->value rather be ->toString() ? ->value alone doesn't
> > really
> > say much. I'm on the fence here though, as there is already
> > (internally) a ->__toString() method to make the (string) cast
> > work.
>
> What is the main difference between getting a read-only property with
> `->value` and getting the value using a method?
Feeling :-) Do we have precedence with other extension's objects
perhaps already?
> > - Would it make sense to have "floor" and "ceil" to also have a scale,
> > or precision? Or would developers instead have to use "round" in
> > that case?
>
> > - Which rounding modes are supported with "round", the same ones as
> > the
> > normal round() function?
>
> `bcfloor` and `bcceil` originally have no scale specification. This is
> because the result is always a string representing an integer value.
> And since the supported round-mode is the same as standard-round,
> `ROUND_FLOOR` and `ROUND_CEILING` are also supported. Therefore, if
> want to obtain floor or ceil behavior with a specified scale, I
> recommend specifying the mode as round.
OK. That makes sense.
> > - In this example, what would $result->scale show? (Perhaps add that to
> > the example?):
> >
> > <?php
> > $num = new BcNum('1.23', 2);
> > $result = $num + '1.23456';
> > $result->value; // '2.46456'
> > $result->scale; // ??
>
> In this case, `$result->scale` will be `'5'`. I added this to the RFC.
Great.
> > - Exceptions
> >
> > The RFC does not mention which exceptions can be thrown. Is it just
> > the one? It might be beneficial to *do* have a new exception
> > hierarchy.
>
>
> As far as I know right now, following exceptions can be thrown:
>
> - Value error when a string that is invalid as a number is used in a
> constructor, calculation method, or operation
> - Divide by 0 error (include Modulo by zero)
>
> I was thinking that it would be a bad idea to increase the number of
> classes without thinking, and was planning to use general exceptions,
> but would it be better to use dedicated exceptions?
It's what we did for the Date extension, and the Random extension, but
in this case, it's probably not needed as you say.
> By the way, generally when implementing such exceptions in userland,
> value errors and divide-by-zero errors are probably defined as
> separate classes, but should they be separated in this case?
For that, yes. ValueErrors should be distinct from DivideByZeroError — I
think we do have both of those already:
php -r 'echo 8/0;'
Fatal error: Uncaught DivisionByZeroError: Division by zero in Command
line code on line 1
From the docs for ValueError:
"A ValueError is thrown when the type of an argument is correct but the
value of it is incorrect."
From the docs for DivisionByZeroError:
" DivisionByZeroError is thrown when an attempt is made to divide a
number by zero. "
Subclassing these for BCMath objects seems unnecessary therefore.
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
mastodon: @derickr@phpc.social @xdebug@phpc.social
