Some minor refactoring (#1923)

* Some minor refactoring

src/PhpSpreadsheet/Calculation/Calculation.php

@@ -3345,18 +3345,15 @@ class Calculation
     }
 
     /**
-     * @param string $cellReference
      * @param mixed $cellValue
-     *
-     * @return bool
      */
-    public function getValueFromCache($cellReference, &$cellValue)
+    public function getValueFromCache(string $cellReference, &$cellValue): bool
     {
+        $this->debugLog->writeDebugLog("Testing cache value for cell {$cellReference}");
         // Is calculation cacheing enabled?
-        // Is the value present in calculation cache?
+        // If so, is the required value present in calculation cache?
-        $this->debugLog->writeDebugLog('Testing cache value for cell ', $cellReference);
         if (($this->calculationCacheEnabled) && (isset($this->calculationCache[$cellReference]))) {
-            $this->debugLog->writeDebugLog('Retrieving value for cell ', $cellReference, ' from cache');
+            $this->debugLog->writeDebugLog("Retrieving value for cell {$cellReference} from cache");
             // Return the cached result
 
             $cellValue = $this->calculationCache[$cellReference];
@@ -3418,7 +3415,7 @@ class Calculation
         if (($cellID !== null) && ($this->getValueFromCache($wsCellReference, $cellValue))) {
             return $cellValue;
         }
-        $this->debugLog->writeDebugLog('Evaluating formula for cell ', $wsCellReference);
+        $this->debugLog->writeDebugLog("Evaluating formula for cell {$wsCellReference}");
 
         if (($wsTitle[0] !== "\x00") && ($this->cyclicReferenceStack->onStack($wsCellReference))) {
             if ($this->cyclicFormulaCount <= 0) {
@@ -3440,7 +3437,7 @@ class Calculation
             }
         }
 
-        $this->debugLog->writeDebugLog('Formula for cell ', $wsCellReference, ' is ', $formula);
+        $this->debugLog->writeDebugLog("Formula for cell {$wsCellReference} is {$formula}");
         //    Parse the formula onto the token stack and calculate the value
         $this->cyclicReferenceStack->push($wsCellReference);
 
@@ -4810,23 +4807,22 @@ class Calculation
      * @param mixed $operand1
      * @param mixed $operand2
      * @param string $operation
-     * @param bool $recursingArrays
      *
-     * @return mixed
+     * @return array
      */
-    private function executeBinaryComparisonOperation($cellID, $operand1, $operand2, $operation, Stack &$stack, $recursingArrays = false)
+    private function executeArrayComparison($cellID, $operand1, $operand2, $operation, Stack &$stack, bool $recursingArrays)
     {
-        //    If we're dealing with matrix operations, we want a matrix result
-        if ((is_array($operand1)) || (is_array($operand2))) {
         $result = [];
-            if ((is_array($operand1)) && (!is_array($operand2))) {
+        if (!is_array($operand2)) {
+            // Operand 1 is an array, Operand 2 is a scalar
             foreach ($operand1 as $x => $operandData) {
                 $this->debugLog->writeDebugLog('Evaluating Comparison ', $this->showValue($operandData), ' ', $operation, ' ', $this->showValue($operand2));
                 $this->executeBinaryComparisonOperation($cellID, $operandData, $operand2, $operation, $stack);
                 $r = $stack->pop();
                 $result[$x] = $r['value'];
             }
-            } elseif ((!is_array($operand1)) && (is_array($operand2))) {
+        } elseif (!is_array($operand1)) {
+            // Operand 1 is a scalar, Operand 2 is an array
             foreach ($operand2 as $x => $operandData) {
                 $this->debugLog->writeDebugLog('Evaluating Comparison ', $this->showValue($operand1), ' ', $operation, ' ', $this->showValue($operandData));
                 $this->executeBinaryComparisonOperation($cellID, $operand1, $operandData, $operation, $stack);
@@ -4834,6 +4830,7 @@ class Calculation
                 $result[$x] = $r['value'];
             }
         } else {
+            // Operand 1 and Operand 2 are both arrays
             if (!$recursingArrays) {
                 self::checkMatrixOperands($operand1, $operand2, 2);
             }
@@ -4852,6 +4849,22 @@ class Calculation
         return $result;
     }
 
+    /**
+     * @param null|string $cellID
+     * @param mixed $operand1
+     * @param mixed $operand2
+     * @param string $operation
+     * @param bool $recursingArrays
+     *
+     * @return mixed
+     */
+    private function executeBinaryComparisonOperation($cellID, $operand1, $operand2, $operation, Stack &$stack, $recursingArrays = false)
+    {
+        //    If we're dealing with matrix operations, we want a matrix result
+        if ((is_array($operand1)) || (is_array($operand2))) {
+            return $this->executeArrayComparison($cellID, $operand1, $operand2, $operation, $stack, $recursingArrays);
+        }
+
         //    Simple validate the two operands if they are string values
         if (is_string($operand1) && $operand1 > '' && $operand1[0] == self::FORMULA_STRING_QUOTE) {
             $operand1 = self::unwrapResult($operand1);
@@ -4863,10 +4876,10 @@ class Calculation
         // Use case insensitive comparaison if not OpenOffice mode
         if (Functions::getCompatibilityMode() != Functions::COMPATIBILITY_OPENOFFICE) {
             if (is_string($operand1)) {
-                $operand1 = strtoupper($operand1);
+                $operand1 = Shared\StringHelper::strToUpper($operand1);
             }
             if (is_string($operand2)) {
-                $operand2 = strtoupper($operand2);
+                $operand2 = Shared\StringHelper::strToUpper($operand2);
             }
         }
 

src/PhpSpreadsheet/Calculation/Logical/Conditional.php

@@ -83,11 +83,11 @@ class Conditional
             $targetValue = Functions::flattenSingleValue($arguments[0]);
             $argc = count($arguments) - 1;
             $switchCount = floor($argc / 2);
-            $switchSatisfied = false;
             $hasDefaultClause = $argc % 2 !== 0;
-            $defaultClause = $argc % 2 === 0 ? null : $arguments[count($arguments) - 1];
+            $defaultClause = $argc % 2 === 0 ? null : $arguments[$argc];
 
-            if ($switchCount) {
+            $switchSatisfied = false;
+            if ($switchCount > 0) {
                 for ($index = 0; $index < $switchCount; ++$index) {
                     if ($targetValue == $arguments[$index * 2 + 1]) {
                         $result = $arguments[$index * 2 + 2];
@@ -98,7 +98,7 @@ class Conditional
                 }
             }
 
-            if (!$switchSatisfied) {
+            if ($switchSatisfied !== true) {
                 $result = $hasDefaultClause ? $defaultClause : Functions::NA();
             }
         }
@@ -161,12 +161,14 @@ class Conditional
      */
     public static function IFS(...$arguments)
     {
-        if (count($arguments) % 2 != 0) {
+        $argumentCount = count($arguments);
+
+        if ($argumentCount % 2 != 0) {
             return Functions::NA();
         }
         // We use instance of Exception as a falseValue in order to prevent string collision with value in cell
         $falseValueException = new Exception();
-        for ($i = 0; $i < count($arguments); $i += 2) {
+        for ($i = 0; $i < $argumentCount; $i += 2) {
             $testValue = ($arguments[$i] === null) ? '' : Functions::flattenSingleValue($arguments[$i]);
             $returnIfTrue = ($arguments[$i + 1] === null) ? '' : Functions::flattenSingleValue($arguments[$i + 1]);
             $result = self::statementIf($testValue, $returnIfTrue, $falseValueException);

src/PhpSpreadsheet/Cell/Coordinate.php

@@ -339,7 +339,8 @@ abstract class Coordinate
 
     private static function processRangeSetOperators(array $operators, array $cells): array
     {
-        for ($offset = 0; $offset < count($operators); ++$offset) {
+        $operatorCount = count($operators);
+        for ($offset = 0; $offset < $operatorCount; ++$offset) {
             $operator = $operators[$offset];
             if ($operator !== ' ') {
                 continue;
@@ -350,6 +351,7 @@ abstract class Coordinate
             $operators = array_values($operators);
             $cells = array_values($cells);
             --$offset;
+            --$operatorCount;
         }
 
         return $cells;

src/PhpSpreadsheet/Document/Properties.php

@@ -506,49 +506,33 @@ class Properties
         switch ($propertyType) {
             case 'empty':     //    Empty
                 return '';
-
-                break;
             case 'null':      //    Null
                 return null;
-
-                break;
             case 'i1':        //    1-Byte Signed Integer
             case 'i2':        //    2-Byte Signed Integer
             case 'i4':        //    4-Byte Signed Integer
             case 'i8':        //    8-Byte Signed Integer
             case 'int':       //    Integer
                 return (int) $propertyValue;
-
-                break;
             case 'ui1':       //    1-Byte Unsigned Integer
             case 'ui2':       //    2-Byte Unsigned Integer
             case 'ui4':       //    4-Byte Unsigned Integer
             case 'ui8':       //    8-Byte Unsigned Integer
             case 'uint':      //    Unsigned Integer
                 return abs((int) $propertyValue);
-
-                break;
             case 'r4':        //    4-Byte Real Number
             case 'r8':        //    8-Byte Real Number
             case 'decimal':   //    Decimal
                 return (float) $propertyValue;
-
-                break;
             case 'lpstr':     //    LPSTR
             case 'lpwstr':    //    LPWSTR
             case 'bstr':      //    Basic String
                 return $propertyValue;
-
-                break;
             case 'date':      //    Date and Time
             case 'filetime':  //    File Time
                 return strtotime($propertyValue);
-
-                break;
             case 'bool':     //    Boolean
                 return $propertyValue == 'true';
-
-                break;
             case 'cy':       //    Currency
             case 'error':    //    Error Status Code
             case 'vector':   //    Vector
@@ -563,8 +547,6 @@ class Properties
             case 'clsid':    //    Class ID
             case 'cf':       //    Clipboard Data
                 return $propertyValue;
-
-                break;
         }
 
         return $propertyValue;
@@ -584,31 +566,21 @@ class Properties
             case 'ui8':      //    8-Byte Unsigned Integer
             case 'uint':     //    Unsigned Integer
                 return self::PROPERTY_TYPE_INTEGER;
-
-                break;
             case 'r4':       //    4-Byte Real Number
             case 'r8':       //    8-Byte Real Number
             case 'decimal':  //    Decimal
                 return self::PROPERTY_TYPE_FLOAT;
-
-                break;
             case 'empty':    //    Empty
             case 'null':     //    Null
             case 'lpstr':    //    LPSTR
             case 'lpwstr':   //    LPWSTR
             case 'bstr':     //    Basic String
                 return self::PROPERTY_TYPE_STRING;
-
-                break;
             case 'date':     //    Date and Time
             case 'filetime': //    File Time
                 return self::PROPERTY_TYPE_DATE;
-
-                break;
             case 'bool':     //    Boolean
                 return self::PROPERTY_TYPE_BOOLEAN;
-
-                break;
             case 'cy':       //    Currency
             case 'error':    //    Error Status Code
             case 'vector':   //    Vector
@@ -623,8 +595,6 @@ class Properties
             case 'clsid':    //    Class ID
             case 'cf':       //    Clipboard Data
                 return self::PROPERTY_TYPE_UNKNOWN;
-
-                break;
         }
 
         return self::PROPERTY_TYPE_UNKNOWN;