From ae09dd13e16532d4648d45d0e3bd5ee486b815c9 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Thu, 5 May 2022 16:53:56 +0200 Subject: [PATCH] Update logic in Cell Iterators to use Cell Collection directly where appropriate, bypassing the overhead of additional calls through the worksheet with extra checks and validations --- src/PhpSpreadsheet/Worksheet/CellIterator.php | 10 +++++- .../Worksheet/ColumnCellIterator.php | 33 ++++++++++++------- .../Worksheet/RowCellIterator.php | 26 ++++++++------- .../Worksheet/ColumnCellIteratorTest.php | 2 +- 4 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/PhpSpreadsheet/Worksheet/CellIterator.php b/src/PhpSpreadsheet/Worksheet/CellIterator.php index 444e3b1f..142639ce 100644 --- a/src/PhpSpreadsheet/Worksheet/CellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/CellIterator.php @@ -4,6 +4,7 @@ namespace PhpOffice\PhpSpreadsheet\Worksheet; use Iterator; use PhpOffice\PhpSpreadsheet\Cell\Cell; +use PhpOffice\PhpSpreadsheet\Collection\Cells; /** * @template TKey @@ -18,6 +19,13 @@ abstract class CellIterator implements Iterator */ protected $worksheet; + /** + * Cell Collection to iterate. + * + * @var Cells + */ + protected $cellCollection; + /** * Iterate only existing cells. * @@ -31,7 +39,7 @@ abstract class CellIterator implements Iterator public function __destruct() { // @phpstan-ignore-next-line - $this->worksheet = null; + $this->worksheet = $this->cellCollection = null; } /** diff --git a/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php b/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php index 9d0be5bd..a518f59c 100644 --- a/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php @@ -42,15 +42,16 @@ class ColumnCellIterator extends CellIterator /** * Create a new row iterator. * - * @param Worksheet $subject The worksheet to iterate over + * @param Worksheet $worksheet The worksheet to iterate over * @param string $columnIndex The column that we want to iterate * @param int $startRow The row number at which to start iterating * @param int $endRow Optionally, the row number at which to stop iterating */ - public function __construct(Worksheet $subject, $columnIndex = 'A', $startRow = 1, $endRow = null) + public function __construct(Worksheet $worksheet, $columnIndex = 'A', $startRow = 1, $endRow = null) { // Set subject - $this->worksheet = $subject; + $this->worksheet = $worksheet; + $this->cellCollection = $worksheet->getCellCollection(); $this->columnIndex = Coordinate::columnIndexFromString($columnIndex); $this->resetEnd($endRow); $this->resetStart($startRow); @@ -96,7 +97,10 @@ class ColumnCellIterator extends CellIterator */ public function seek(int $row = 1) { - if ($this->onlyExistingCells && !($this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $row))) { + if ( + $this->onlyExistingCells && + (!$this->cellCollection->has(Coordinate::stringFromColumnIndex($this->columnIndex) . $row)) + ) { throw new PhpSpreadsheetException('In "IterateOnlyExistingCells" mode and Cell does not exist'); } if (($row < $this->startRow) || ($row > $this->endRow)) { @@ -120,7 +124,11 @@ class ColumnCellIterator extends CellIterator */ public function current(): ?Cell { - return $this->worksheet->getCellByColumnAndRow($this->columnIndex, $this->currentRow); + $cellAddress = Coordinate::stringFromColumnIndex($this->columnIndex) . $this->currentRow; + + return $this->cellCollection->has($cellAddress) + ? $this->cellCollection->get($cellAddress) + : $this->worksheet->createNewCell($cellAddress); } /** @@ -136,12 +144,13 @@ class ColumnCellIterator extends CellIterator */ public function next(): void { + $columnAddress = Coordinate::stringFromColumnIndex($this->columnIndex); do { ++$this->currentRow; } while ( ($this->onlyExistingCells) && - (!$this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $this->currentRow)) && - ($this->currentRow <= $this->endRow) + ($this->currentRow <= $this->endRow) && + (!$this->cellCollection->has($columnAddress . $this->currentRow)) ); } @@ -150,12 +159,13 @@ class ColumnCellIterator extends CellIterator */ public function prev(): void { + $columnAddress = Coordinate::stringFromColumnIndex($this->columnIndex); do { --$this->currentRow; } while ( ($this->onlyExistingCells) && - (!$this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $this->currentRow)) && - ($this->currentRow >= $this->startRow) + ($this->currentRow >= $this->startRow) && + (!$this->cellCollection->has($columnAddress . $this->currentRow)) ); } @@ -173,14 +183,15 @@ class ColumnCellIterator extends CellIterator protected function adjustForExistingOnlyRange(): void { if ($this->onlyExistingCells) { + $columnAddress = Coordinate::stringFromColumnIndex($this->columnIndex); while ( - (!$this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $this->startRow)) && + (!$this->cellCollection->has($columnAddress . $this->startRow)) && ($this->startRow <= $this->endRow) ) { ++$this->startRow; } while ( - (!$this->worksheet->cellExistsByColumnAndRow($this->columnIndex, $this->endRow)) && + (!$this->cellCollection->has($columnAddress . $this->endRow)) && ($this->endRow >= $this->startRow) ) { --$this->endRow; diff --git a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php index a78765bd..5af05072 100644 --- a/src/PhpSpreadsheet/Worksheet/RowCellIterator.php +++ b/src/PhpSpreadsheet/Worksheet/RowCellIterator.php @@ -51,6 +51,7 @@ class RowCellIterator extends CellIterator { // Set subject and row index $this->worksheet = $worksheet; + $this->cellCollection = $worksheet->getCellCollection(); $this->rowIndex = $rowIndex; $this->resetEnd($endColumn); $this->resetStart($startColumn); @@ -97,15 +98,14 @@ class RowCellIterator extends CellIterator */ public function seek(string $column = 'A') { - $columnx = $column; - $column = Coordinate::columnIndexFromString($column); - if ($this->onlyExistingCells && !($this->worksheet->cellExistsByColumnAndRow($column, $this->rowIndex))) { + $columnId = Coordinate::columnIndexFromString($column); + if ($this->onlyExistingCells && !($this->cellCollection->has($column . $this->rowIndex))) { throw new PhpSpreadsheetException('In "IterateOnlyExistingCells" mode and Cell does not exist'); } - if (($column < $this->startColumnIndex) || ($column > $this->endColumnIndex)) { - throw new PhpSpreadsheetException("Column $columnx is out of range ({$this->startColumnIndex} - {$this->endColumnIndex})"); + if (($columnId < $this->startColumnIndex) || ($columnId > $this->endColumnIndex)) { + throw new PhpSpreadsheetException("Column $column is out of range ({$this->startColumnIndex} - {$this->endColumnIndex})"); } - $this->currentColumnIndex = $column; + $this->currentColumnIndex = $columnId; return $this; } @@ -123,7 +123,11 @@ class RowCellIterator extends CellIterator */ public function current(): ?Cell { - return $this->worksheet->getCellByColumnAndRow($this->currentColumnIndex, $this->rowIndex); + $cellAddress = Coordinate::stringFromColumnIndex($this->currentColumnIndex) . $this->rowIndex; + + return $this->cellCollection->has($cellAddress) + ? $this->cellCollection->get($cellAddress) + : $this->worksheet->createNewCell($cellAddress); } /** @@ -141,7 +145,7 @@ class RowCellIterator extends CellIterator { do { ++$this->currentColumnIndex; - } while (($this->onlyExistingCells) && (!$this->worksheet->cellExistsByColumnAndRow($this->currentColumnIndex, $this->rowIndex)) && ($this->currentColumnIndex <= $this->endColumnIndex)); + } while (($this->onlyExistingCells) && (!$this->cellCollection->has(Coordinate::stringFromColumnIndex($this->currentColumnIndex) . $this->rowIndex)) && ($this->currentColumnIndex <= $this->endColumnIndex)); } /** @@ -151,7 +155,7 @@ class RowCellIterator extends CellIterator { do { --$this->currentColumnIndex; - } while (($this->onlyExistingCells) && (!$this->worksheet->cellExistsByColumnAndRow($this->currentColumnIndex, $this->rowIndex)) && ($this->currentColumnIndex >= $this->startColumnIndex)); + } while (($this->onlyExistingCells) && (!$this->cellCollection->has(Coordinate::stringFromColumnIndex($this->currentColumnIndex) . $this->rowIndex)) && ($this->currentColumnIndex >= $this->startColumnIndex)); } /** @@ -176,10 +180,10 @@ class RowCellIterator extends CellIterator protected function adjustForExistingOnlyRange(): void { if ($this->onlyExistingCells) { - while ((!$this->worksheet->cellExistsByColumnAndRow($this->startColumnIndex, $this->rowIndex)) && ($this->startColumnIndex <= $this->endColumnIndex)) { + while ((!$this->cellCollection->has(Coordinate::stringFromColumnIndex($this->startColumnIndex) . $this->rowIndex)) && ($this->startColumnIndex <= $this->endColumnIndex)) { ++$this->startColumnIndex; } - while ((!$this->worksheet->cellExistsByColumnAndRow($this->endColumnIndex, $this->rowIndex)) && ($this->endColumnIndex >= $this->startColumnIndex)) { + while ((!$this->cellCollection->has(Coordinate::stringFromColumnIndex($this->endColumnIndex) . $this->rowIndex)) && ($this->endColumnIndex >= $this->startColumnIndex)) { --$this->endColumnIndex; } } diff --git a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php index b9e18e43..1a7ff675 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php @@ -112,7 +112,7 @@ class ColumnCellIteratorTest extends TestCase $iterator->seek(2); } - public function xtestPrevOutOfRange(): void + public function testPrevOutOfRange(): void { $spreadsheet = new Spreadsheet(); $sheet = self::getPopulatedSheet($spreadsheet);