Some refactoring to extract the logic for calculating the CF operand tokens and size, to reduce duplicated code

Author: MarkBaker

src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php

@@ -0,0 +1,76 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheet\Writer\Xls;
+
+use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
+use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard;
+
+class ConditionalHelper
+{
+    /**
+     * Formula parser.
+     *
+     * @var Parser
+     */
+    protected $parser;
+
+    /**
+     * @var mixed
+     */
+    protected $condition;
+
+    /**
+     * @var string
+     */
+    protected $cellRange;
+
+    /**
+     * @var null|string
+     */
+    protected $tokens;
+
+    /**
+     * @var int
+     */
+    protected $size;
+
+    public function __construct(Parser $parser)
+    {
+        $this->parser = $parser;
+    }
+
+    /**
+     * @param mixed $condition
+     */
+    public function processCondition($condition, string $cellRange): void
+    {
+        $this->condition = $condition;
+        $this->cellRange = $cellRange;
+
+        if (is_int($condition) || is_float($condition)) {
+            $this->size = ($condition <= 65535 ? 3 : 0x0000);
+            $this->tokens = pack('Cv', 0x1E, $condition);
+        } else {
+            try {
+                $formula = Wizard\WizardAbstract::reverseAdjustCellRef((string) $condition, $cellRange);
+                $this->parser->parse($formula);
+                $this->tokens = $this->parser->toReversePolish();
+                $this->size = strlen($this->tokens);
+            } catch (PhpSpreadsheetException $e) {
+                var_dump("PARSER EXCEPTION: {$e->getMessage()}");
+                $this->tokens = null;
+                $this->size = 0;
+            }
+        }
+    }
+
+    public function tokens(): ?string
+    {
+        return $this->tokens;
+    }
+
+    public function size(): int
+    {
+        return $this->size;
+    }
+}

src/PhpSpreadsheet/Writer/Xls/Worksheet.php

@@ -13,7 +13,6 @@
 use PhpOffice\PhpSpreadsheet\Style\Border;
 use PhpOffice\PhpSpreadsheet\Style\Color;
 use PhpOffice\PhpSpreadsheet\Style\Conditional;
-use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard;
 use PhpOffice\PhpSpreadsheet\Style\Protection;
 use PhpOffice\PhpSpreadsheet\Worksheet\PageSetup;
 use PhpOffice\PhpSpreadsheet\Worksheet\SheetView;
@@ -548,6 +547,8 @@ public function close(): void
 
     private function writeConditionalFormatting(): void
     {
+        $conditionalFormulaHelper = new ConditionalHelper($this->parser);
+
         $arrConditionalStyles = $this->phpSheet->getConditionalStylesCollection();
         if (!empty($arrConditionalStyles)) {
             $arrConditional = [];
@@ -570,7 +571,7 @@ private function writeConditionalFormatting(): void
                             $arrConditional[$conditional->getHashCode()] = true;
 
                             // Write CFRULE record
-                            $this->writeCFRule($conditional, $cellCoordinate);
+                            $this->writeCFRule($conditionalFormulaHelper, $conditional, $cellCoordinate);
                         }
                     }
                 }
@@ -2780,8 +2781,11 @@ private function writePageLayoutView(): void
     /**
      * Write CFRule Record.
      */
-    private function writeCFRule(Conditional $conditional, string $cellRange): void
-    {
+    private function writeCFRule(
+        ConditionalHelper $conditionalFormulaHelper,
+        Conditional $conditional,
+        string $cellRange
+    ): void {
         $record = 0x01B1; // Record identifier
         $type = null;  //  Type of the CF
         $operatorType = null;   //  Comparison operator
@@ -2839,53 +2843,17 @@ private function writeCFRule(Conditional $conditional, string $cellRange): void
         $operand1 = null;
         $operand2 = null;
 
-        if ($numConditions == 1) {
-            if (is_int($arrConditions[0]) || is_float($arrConditions[0])) {
-                $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000);
-                $operand1 = pack('Cv', 0x1E, $arrConditions[0]);
-            } else {
-                try {
-                    $formula1 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[0], $cellRange);
-                    $this->parser->parse($formula1);
-                    $formula1 = $this->parser->toReversePolish();
-                    $szValue1 = strlen($formula1);
-                } catch (PhpSpreadsheetException $e) {
-                    var_dump("PARSER EXCEPTION: {$e->getMessage()}");
-                    $formula1 = null;
-                }
-                $operand1 = $formula1;
-            }
-        } elseif ($numConditions == 2 && ($conditional->getOperatorType() == Conditional::OPERATOR_BETWEEN)) {
-            if (is_int($arrConditions[0]) || is_float($arrConditions[0])) {
-                $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000);
-                $operand1 = pack('Cv', 0x1E, $arrConditions[0]);
-            } else {
-                try {
-                    $formula1 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[0], $cellRange);
-                    $this->parser->parse($formula1);
-                    $formula1 = $this->parser->toReversePolish();
-                    $szValue1 = strlen($formula1);
-                } catch (PhpSpreadsheetException $e) {
-                    var_dump("PARSER EXCEPTION: {$e->getMessage()}");
-                    $formula1 = null;
-                }
-                $operand1 = $formula1;
-            }
-            if (is_int($arrConditions[1]) || is_float($arrConditions[1])) {
-                $szValue2 = ($arrConditions[1] <= 65535 ? 3 : 0x0000);
-                $operand2 = pack('Cv', 0x1E, $arrConditions[1]);
-            } else {
-                try {
-                    $formula2 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[1], $cellRange);
-                    $this->parser->parse($formula2);
-                    $formula2 = $this->parser->toReversePolish();
-                    $szValue2 = strlen($formula2);
-                } catch (PhpSpreadsheetException $e) {
-                    var_dump("PARSER EXCEPTION: {$e->getMessage()}");
-                    $formula2 = null;
-                }
-                $operand2 = $formula2;
-            }
+        if ($numConditions === 1) {
+            $conditionalFormulaHelper->processCondition($arrConditions[0], $cellRange);
+            $szValue1 = $conditionalFormulaHelper->size();
+            $operand1 = $conditionalFormulaHelper->tokens();
+        } elseif ($numConditions === 2 && ($conditional->getOperatorType() === Conditional::OPERATOR_BETWEEN)) {
+            $conditionalFormulaHelper->processCondition($arrConditions[0], $cellRange);
+            $szValue1 = $conditionalFormulaHelper->size();
+            $operand1 = $conditionalFormulaHelper->tokens();
+            $conditionalFormulaHelper->processCondition($arrConditions[1], $cellRange);
+            $szValue2 = $conditionalFormulaHelper->size();
+            $operand2 = $conditionalFormulaHelper->tokens();
         }
 
         // $flags : Option flags