[PHP 8.4] Add bcdivmod
#503
Conversation
| @@ -60,3 +60,7 @@ function mb_ltrim(string $string, ?string $characters = null, ?string $encoding | |||
| function mb_rtrim(string $string, ?string $characters = null, ?string $encoding = null): string { return p\Php84::mb_rtrim($string, $characters, $encoding); } | |||
| } | |||
| } | |||
|
|
|||
| if (function_exists('bcdiv')) { | |||
| function bcdivmod(string $num1, string $num2, ?int $scale = null): array {return p\Php84::bcdivmod($num1, $num2, $scale); } | |||
There was a problem hiding this comment.
such polyfill function should be defined only when the bcmath extension is loaded (similar to how we deal with mbstring new functions just above)
There was a problem hiding this comment.
I was thinking the function_exists('bcdiv') call will check for this, and will also play nicely with any potential polyfills that may declare a bcdiv function. If you think, I can update this with a full extension_loaded call.
There was a problem hiding this comment.
oh, I missed that this was checking bcdiv and not bcdivmod. But then, it misses the check for bcdivmod (all polyfilled functions need to be guarded to avoid double function definition).
and for the detection of bcmath, please use extension_loaded for consistency with our other polyfills. Thus, a extension_loaded call is fully resolved at compile-time by OPCache.
| * | ||
| * @dataProvider bcDivModProvider | ||
| */ | ||
| public function testBcDivMod(string $num1, string $num2, ?int $scale = null, array $expected): void |
There was a problem hiding this comment.
this test needs @requires extension bcmath as the implementation would break without that extension
There was a problem hiding this comment.
Brilliant, thank you.
| if ($num2 === '0') { | ||
| throw new \DivisionByZeroError('Division by zero'); | ||
| } | ||
|
|
||
| if (!is_numeric($num1)) { | ||
| throw new \ValueError('Argument #1 ($num1) is not well-formed'); | ||
| } | ||
|
|
||
| if (!is_numeric($num2)) { | ||
| throw new \ValueError('Argument #2 ($num2) is not well-formed'); | ||
| } |
There was a problem hiding this comment.
Those two errors are not available on PHP 7. I suggest to emit a E_USER_WARNING and return null instead if we're on PHP < 8.
|
|
||
| public static function bcDivModProvider(): iterable | ||
| { | ||
| yield ['10', '10', null, ['1', '0']]; |
There was a problem hiding this comment.
A couple more tests would be nice. You're only testing the simplest path.
Also, beware that by setting the scale to null, your test depends on the global bcmath.scale setting. Currently, the test will fail if it's run in an environment where that setting is not 0.
|
@Ayesh can you update your PR ? |
https://wiki.php.net/rfc/add_bcdivmod_to_bcmath