Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigNumber::of performance update #77

Merged
merged 5 commits into from
Nov 29, 2023
Merged

BigNumber::of performance update #77

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1a3ebd6
BigNumber::of performance update
SebastienDug Nov 10, 2023
8fb9e4b
fix for return type
SebastienDug Nov 10, 2023
3f69824
update
SebastienDug Nov 10, 2023
42daee9
adjustments as per request
SebastienDug Nov 27, 2023
0ee0739
merge conflict resolved
SebastienDug Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 87 additions & 60 deletions src/BigNumber.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,27 @@
abstract class BigNumber implements \Serializable, \JsonSerializable
{
/**
* The regular expression used to parse integer, decimal and rational numbers.
* The regular expression used to parse integer or decimal numbers.
*/
private const PARSE_REGEXP =
private const PARSE_REGEXP_NUMERICAL =
'/^' .
'(?<sign>[\-\+])?' .
'(?:' .
'(?:' .
'(?<integral>[0-9]+)?' .
'(?<point>\.)?' .
'(?<fractional>[0-9]+)?' .
'(?:[eE](?<exponent>[\-\+]?[0-9]+))?' .
')|(?:' .
'(?<numerator>[0-9]+)' .
'\/?' .
'(?<denominator>[0-9]+)' .
')' .
')' .
'(?<integral>[0-9]+)?' .
'(?<point>\.)?' .
'(?<fractional>[0-9]+)?' .
'(?:[eE](?<exponent>[\-\+]?[0-9]+))?' .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'(?<integral>[0-9]+)?' .
'(?<point>\.)?' .
'(?<fractional>[0-9]+)?' .
'(?:[eE](?<exponent>[\-\+]?[0-9]+))?' .
'(?<integral>[0-9]+)?' .
'(?<point>\.)?' .
'(?<fractional>[0-9]+)?' .
'(?:[eE](?<exponent>[\-\+]?[0-9]+))?' .

'$/';

/**
* The regular expression used to parse rational numbers.
*/
private const PARSE_REGEXP_RATIONAL =
'/^' .
'(?<sign>[\-\+])?' .
'(?<numerator>[0-9]+)' .
'\/?' .
'(?<denominator>[0-9]+)' .
'$/';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'/^' .
'(?<sign>[\-\+])?' .
'(?<numerator>[0-9]+)' .
'\/?' .
'(?<denominator>[0-9]+)' .
'$/';
'/^' .
'(?<sign>[\-\+])?' .
'(?<numerator>[0-9]+)' .
'\/?' .
'(?<denominator>[0-9]+)' .
'$/';

/**
* Creates a BigNumber of the given value.
*
Expand Down Expand Up @@ -67,24 +69,17 @@ public static function of(BigNumber|int|float|string $value) : BigNumber
$value = (string) $value;
}

$throw = static function() use ($value) : void {
throw new NumberFormatException(\sprintf(
'The given value "%s" does not represent a valid number.',
$value
));
};

if (\preg_match(self::PARSE_REGEXP, $value, $matches) !== 1) {
$throw();
}

$getMatch = static fn(string $value): ?string => (($matches[$value] ?? '') !== '') ? $matches[$value] : null;
if (str_contains($value, '/')) {
// Rational number
if (\preg_match(self::PARSE_REGEXP_RATIONAL, $value, $matches, PREG_UNMATCHED_AS_NULL) !== 1) {
self::throwException($value);
}

$sign = $getMatch('sign');
$numerator = $getMatch('numerator');
$denominator = $getMatch('denominator');
$sign = $matches['sign'];
$numerator = $matches['numerator'];
$denominator = $matches['denominator'];

if ($numerator !== null) {
assert($numerator !== null);
assert($denominator !== null);

if ($sign !== null) {
Expand All @@ -103,46 +98,63 @@ public static function of(BigNumber|int|float|string $value) : BigNumber
new BigInteger($denominator),
false
);
}
} else {
// Integer or decimal number
if (\preg_match(self::PARSE_REGEXP_NUMERICAL, $value, $matches, PREG_UNMATCHED_AS_NULL) !== 1) {
self::throwException($value);
}

$point = $getMatch('point');
$integral = $getMatch('integral');
$fractional = $getMatch('fractional');
$exponent = $getMatch('exponent');
$sign = $matches['sign'];
$point = $matches['point'];
$integral = $matches['integral'];
$fractional = $matches['fractional'];
$exponent = $matches['exponent'];

if ($integral === null && $fractional === null) {
$throw();
}
if ($integral === null && $fractional === null) {
self::throwException($value);
}

if ($integral === null) {
$integral = '0';
}
if ($integral === null) {
$integral = '0';
}

if ($point !== null || $exponent !== null) {
$fractional = ($fractional ?? '');
$exponent = ($exponent !== null) ? (int) $exponent : 0;
if ($point !== null || $exponent !== null) {
$fractional = ($fractional ?? '');
$exponent = ($exponent !== null) ? (int)$exponent : 0;

if ($exponent === PHP_INT_MIN || $exponent === PHP_INT_MAX) {
throw new NumberFormatException('Exponent too large.');
}
if ($exponent === PHP_INT_MIN || $exponent === PHP_INT_MAX) {
throw new NumberFormatException('Exponent too large.');
}

$unscaledValue = self::cleanUp(($sign ?? ''). $integral . $fractional);
$unscaledValue = self::leanCleanUp(($sign ?? ''), $integral . $fractional);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$unscaledValue = self::leanCleanUp(($sign ?? ''), $integral . $fractional);
$unscaledValue = self::cleanUp($sign, $integral . $fractional);

After leanCleanUp has been renamed to cleanUp.


$scale = \strlen($fractional) - $exponent;
$scale = \strlen($fractional) - $exponent;

if ($scale < 0) {
if ($unscaledValue !== '0') {
$unscaledValue .= \str_repeat('0', - $scale);
if ($scale < 0) {
if ($unscaledValue !== '0') {
$unscaledValue .= \str_repeat('0', -$scale);
}
$scale = 0;
}
$scale = 0;

return new BigDecimal($unscaledValue, $scale);
}

return new BigDecimal($unscaledValue, $scale);
}
$integral = self::leanCleanUp(($sign ?? ''), $integral);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$integral = self::leanCleanUp(($sign ?? ''), $integral);
$integral = self::cleanUp($sign, $integral);

After leanCleanUp has been renamed to cleanUp.


$integral = self::cleanUp(($sign ?? '') . $integral);
return new BigInteger($integral);
}
}

return new BigInteger($integral);
/**
* Throws a NumberFormatException for the given value.
* @psalm-pure
*/
protected static function throwException(string $value) : void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces + private:

Suggested change
/**
* Throws a NumberFormatException for the given value.
* @psalm-pure
*/
protected static function throwException(string $value) : void {
/**
* Throws a NumberFormatException for the given value.
*
* @psalm-pure
*/
private static function throwException(string $value) : void {

throw new NumberFormatException(\sprintf(
'The given value "%s" does not represent a valid number.',
$value
));
}

/**
Expand Down Expand Up @@ -325,11 +337,26 @@ private static function cleanUp(string $number) : string
return '0';
}

if ($firstChar === '-') {
return '-' . $number;
return $firstChar === '-' ? '-' . $number : $number;
}

/**
* Removes optional leading zeros and applies sign if needed(- for negatives).
*
* @param string $number The number, validated as a non-empty string of digits
* @param string $sign The sign, + or -
BenMorel marked this conversation as resolved.
Show resolved Hide resolved
*
* @psalm-pure
*/
private static function leanCleanUp(string $sign, string $number) : string
BenMorel marked this conversation as resolved.
Show resolved Hide resolved
{
$number = \ltrim($number, '0');

if ($number === '') {
return '0';
}

return $number;
return $sign === '-' ? '-' . $number : $number;
}

/**
Expand Down
Loading