Hi Derick, > - You've picked as class name "BcNum". Following > our naming guidelines, that probably should be \BCMath\Num (or > \BC\Num, but that is less descriptive): > > https://github.com/php/policies/blob/main/coding-standards-and-naming.rst#namespaces-in-extensions > > The reason it *should* have "BC" is that it comes from "Basic > Calculator" (https://www.php.net/manual/en/book.bc.php#118203)
I re-read the namespace RFC again. I also re-read the RFC regarding class naming conventions. https://wiki.php.net/rfc/namespaces_in_bundled_extensions https://wiki.php.net/rfc/class-naming There's no need for the namespace to follow class naming conventions, but the acronym doesn't seem to need to be pascal-case anyway (I remembered it incorrectly). However, the RFC states that the extension's namespace must match the extension name, so it seems correct in this case for the namespace to be `BCMath`. And indeed, looking at the example in the namespace RFC, `BCMath\Number` might be appropriate in this case (I think I was sleepy yesterday). I changed `BcNum` to `BCMath\Number` in my RFC. > - Should ->value rather be ->toString() ? ->value alone doesn't really > say much. I'm on the fence here though, as there is already > (internally) a ->__toString() method to make the (string) cast work. What is the main difference between getting a read-only property with `->value` and getting the value using a method? > - Would it make sense to have "floor" and "ceil" to also have a scale, > or precision? Or would developers instead have to use "round" in that > case? > - Which rounding modes are supported with "round", the same ones as the > normal round() function? `bcfloor` and `bcceil` originally have no scale specification. This is because the result is always a string representing an integer value. And since the supported round-mode is the same as standard-round, `ROUND_FLOOR` and `ROUND_CEILING` are also supported. Therefore, if want to obtain floor or ceil behavior with a specified scale, I recommend specifying the mode as round. > - In this example, what would $result->scale show? (Perhaps add that to > the example?): > > <?php > $num = new BcNum('1.23', 2); > $result = $num + '1.23456'; > $result->value; // '2.46456' > $result->scale; // ?? In this case, `$result->scale` will be `'5'`. I added this to the RFC. > - Exceptions > > The RFC does not mention which exceptions can be thrown. Is it just > the one? It might be beneficial to *do* have a new exception > hierarchy. As far as I know right now, following exceptions can be thrown: - Value error when a string that is invalid as a number is used in a constructor, calculation method, or operation - Divide by 0 error (include Modulo by zero) I was thinking that it would be a bad idea to increase the number of classes without thinking, and was planning to use general exceptions, but would it be better to use dedicated exceptions? By the way, generally when implementing such exceptions in userland, value errors and divide-by-zero errors are probably defined as separate classes, but should they be separated in this case? Regards. Saki
