Skip to content

Commit

Permalink
Support 'Forms' for ROMAN Function (#1828)
Browse files Browse the repository at this point in the history
* Support 'Forms' for ROMAN Function

This seems like an exceptionally silly thing for MS to have implemented
(Wikipedia on Roman Numerals: "There is no indication this is anything
other than an invention by the programmer").
Nevertheless, we can, and therefore probably should, implement it.

Not that I can implement it by an algorithm - Excel describes the various extra
styles as "more concise", "more concise", "more concise", and "simplified".
Nevertheless, since the universe of potential calls is relatively small,
it can be implemented as a table of values where the new forms would return
a different value than "classic". This table is relatively large, so I have
put it its own member to avoid overhead when the function is needed.

* Move ROMAN To Its Own Class

See discussion in PR #1837

* PHP 8.1 Deprecations

PHP8.1 Unit tests failed. 1 line fixes are available for
- Shared/Font
- Shared/XMLWriter
- Style/Color
- Writer/HTML

The problem is that an error is also reported for a strcmp at
line 272 of Cell/Cell. Not only does that line not invoke strcmp,
there is no strcmp in all of Cell/Cell, so I don't know what to make
of the error message. Oh well, let's fix what can be fixed.

Still dealing with the mysterious PHP8.1 unit test failure in Cell\Cell,
which seems to have something to do with strcmp. The only uses of
strcmp that I can find in src/ are in Calculation. I can't find any
use of it in test/ or samples/. So, if this doesn't fix the problem,
I may have to give up.
  • Loading branch information
oleibman committed Feb 13, 2021
1 parent cabcfaa commit a24ca09
Show file tree
Hide file tree
Showing 10 changed files with 978 additions and 79 deletions.
29 changes: 21 additions & 8 deletions src/PhpSpreadsheet/Calculation/Calculation.php
Original file line number Diff line number Diff line change
Expand Up @@ -2107,7 +2107,7 @@ class Calculation
],
'ROMAN' => [
'category' => Category::CATEGORY_MATH_AND_TRIG,
'functionCall' => [MathTrig::class, 'ROMAN'],
'functionCall' => [MathTrig\Roman::class, 'funcRoman'],
'argumentCount' => '1,2',
],
'ROUND' => [
Expand Down Expand Up @@ -4895,7 +4895,7 @@ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2,
if (is_numeric($operand1) && is_numeric($operand2)) {
$result = (abs($operand1 - $operand2) < $this->delta);
} else {
$result = strcmp($operand1, $operand2) == 0;
$result = $this->strcmpAllowNull($operand1, $operand2) == 0;
}

break;
Expand All @@ -4906,7 +4906,7 @@ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2,
} elseif ($useLowercaseFirstComparison) {
$result = $this->strcmpLowercaseFirst($operand1, $operand2) >= 0;
} else {
$result = strcmp($operand1, $operand2) >= 0;
$result = $this->strcmpAllowNull($operand1, $operand2) >= 0;
}

break;
Expand All @@ -4917,7 +4917,7 @@ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2,
} elseif ($useLowercaseFirstComparison) {
$result = $this->strcmpLowercaseFirst($operand1, $operand2) <= 0;
} else {
$result = strcmp($operand1, $operand2) <= 0;
$result = $this->strcmpAllowNull($operand1, $operand2) <= 0;
}

break;
Expand All @@ -4926,7 +4926,7 @@ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2,
if (is_numeric($operand1) && is_numeric($operand2)) {
$result = (abs($operand1 - $operand2) > 1E-14);
} else {
$result = strcmp($operand1, $operand2) != 0;
$result = $this->strcmpAllowNull($operand1, $operand2) != 0;
}

break;
Expand All @@ -4943,8 +4943,8 @@ private function executeBinaryComparisonOperation($cellID, $operand1, $operand2,
/**
* Compare two strings in the same way as strcmp() except that lowercase come before uppercase letters.
*
* @param string $str1 First string value for the comparison
* @param string $str2 Second string value for the comparison
* @param null|string $str1 First string value for the comparison
* @param null|string $str2 Second string value for the comparison
*
* @return int
*/
Expand All @@ -4953,7 +4953,20 @@ private function strcmpLowercaseFirst($str1, $str2)
$inversedStr1 = Shared\StringHelper::strCaseReverse($str1);
$inversedStr2 = Shared\StringHelper::strCaseReverse($str2);

return strcmp($inversedStr1, $inversedStr2);
return strcmp($inversedStr1 ?? '', $inversedStr2 ?? '');
}

/**
* PHP8.1 deprecates passing null to strcmp.
*
* @param null|string $str1 First string value for the comparison
* @param null|string $str2 Second string value for the comparison
*
* @return int
*/
private function strcmpAllowNull($str1, $str2)
{
return strcmp($str1 ?? '', $str2 ?? '');
}

/**
Expand Down
48 changes: 15 additions & 33 deletions src/PhpSpreadsheet/Calculation/MathTrig.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,6 @@ private static function factors($value)
return [(int) $value];
}

private static function romanCut($num, $n)
{
return ($num - ($num % $n)) / $n;
}

private static function strSplit(string $roman): array
{
$rslt = str_split($roman);
Expand Down Expand Up @@ -995,36 +990,23 @@ public static function RAND($min = 0, $max = 0)
return mt_rand($min, $max);
}

/**
* ROMAN.
*
* Converts a number to Roman numeral
*
* @Deprecated 2.0.0 Use the funcRoman method in the MathTrig\Roman class instead
*
* @param mixed $aValue Number to convert
* @param mixed $style Number indicating one of five possible forms
*
* @return string Roman numeral, or a string containing an error
*
* @codeCoverageIgnore
*/
public static function ROMAN($aValue, $style = 0)
{
$aValue = Functions::flattenSingleValue($aValue);
$style = ($style === null) ? 0 : (int) Functions::flattenSingleValue($style);
if ((!is_numeric($aValue)) || ($aValue < 0) || ($aValue >= 4000)) {
return Functions::VALUE();
}
$aValue = (int) $aValue;
if ($aValue == 0) {
return '';
}

$mill = ['', 'M', 'MM', 'MMM', 'MMMM', 'MMMMM'];
$cent = ['', 'C', 'CC', 'CCC', 'CD', 'D', 'DC', 'DCC', 'DCCC', 'CM'];
$tens = ['', 'X', 'XX', 'XXX', 'XL', 'L', 'LX', 'LXX', 'LXXX', 'XC'];
$ones = ['', 'I', 'II', 'III', 'IV', 'V', 'VI', 'VII', 'VIII', 'IX'];

$roman = '';
while ($aValue > 5999) {
$roman .= 'M';
$aValue -= 1000;
}
$m = self::romanCut($aValue, 1000);
$aValue %= 1000;
$c = self::romanCut($aValue, 100);
$aValue %= 100;
$t = self::romanCut($aValue, 10);
$aValue %= 10;

return $roman . $mill[$m] . $cent[$c] . $tens[$t] . $ones[$aValue];
return MathTrig\Roman::funcRoman($aValue, $style);
}

/**
Expand Down

0 comments on commit a24ca09

Please sign in to comment.