Skip to content

Commit 1924f3c

Browse files
author
MarkBaker
committed
Improved memory performance for getSortedCoordinates() in the Cell Collection; with minimal execution time overhead.
This is an important method, as it's called by the Reference Helper and for the Writer save() method; so users should see that improvement.
1 parent 533f41f commit 1924f3c

File tree

2 files changed

+54
-39
lines changed

2 files changed

+54
-39
lines changed

src/PhpSpreadsheet/Collection/Cells.php

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Generator;
66
use PhpOffice\PhpSpreadsheet\Cell\Cell;
7+
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
78
use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
89
use PhpOffice\PhpSpreadsheet\Settings;
910
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
@@ -150,41 +151,16 @@ public function getCoordinates()
150151
public function getSortedCoordinates()
151152
{
152153
$sortKeys = [];
153-
foreach ($this->getCoordinates() as $coord) {
154-
sscanf($coord, '%[A-Z]%d', $column, $row);
155-
$sortKeys[sprintf('%09d%3s', $row, $column)] = $coord;
154+
foreach ($this->getCoordinates() as $coordinate) {
155+
sscanf($coordinate, '%[A-Z]%d', $column, $row);
156+
$key = (--$row * 16384) + Coordinate::columnIndexFromString($column);
157+
$sortKeys[$key] = $coordinate;
156158
}
157159
ksort($sortKeys);
158160

159161
return array_values($sortKeys);
160162
}
161163

162-
/**
163-
* Get highest worksheet column and highest row that have cell records.
164-
*
165-
* @return array Highest column name and highest row number
166-
*/
167-
public function getHighestRowAndColumn()
168-
{
169-
// Lookup highest column and highest row
170-
$col = ['A' => '1A'];
171-
$row = [1];
172-
foreach ($this->getCoordinates() as $coord) {
173-
sscanf($coord, '%[A-Z]%d', $c, $r);
174-
$row[$r] = $r;
175-
$col[$c] = strlen($c) . $c;
176-
}
177-
178-
// Determine highest column and row
179-
$highestRow = max($row);
180-
$highestColumn = substr((string) @max($col), 1);
181-
182-
return [
183-
'row' => $highestRow,
184-
'column' => $highestColumn,
185-
];
186-
}
187-
188164
/**
189165
* Return the cell coordinate of the currently active cell object.
190166
*
@@ -219,6 +195,32 @@ public function getCurrentRow()
219195
return (int) $row;
220196
}
221197

198+
/**
199+
* Get highest worksheet column and highest row that have cell records.
200+
*
201+
* @return array Highest column name and highest row number
202+
*/
203+
public function getHighestRowAndColumn()
204+
{
205+
// Lookup highest column and highest row
206+
$columns = ['1A'];
207+
$rows = [1];
208+
foreach ($this->getCoordinates() as $coordinate) {
209+
sscanf($coordinate, '%[A-Z]%d', $column, $row);
210+
$rows[$row] = $rows[$row] ?? $row;
211+
$columns[$column] = $columns[$column] ?? strlen($column) . $column;
212+
}
213+
214+
// Determine highest column and row
215+
$highestRow = max($rows);
216+
$highestColumn = substr((string) max($columns), 1);
217+
218+
return [
219+
'row' => $highestRow,
220+
'column' => $highestColumn,
221+
];
222+
}
223+
222224
/**
223225
* Get highest worksheet column.
224226
*
@@ -239,7 +241,8 @@ public function getHighestColumn($row = null)
239241
if ($r != $row) {
240242
continue;
241243
}
242-
$maxColumn = max($maxColumn, strlen($c) . $c);
244+
$sortableColum = strlen($c) . $c;
245+
$maxColumn = $maxColumn > $sortableColum ? $maxColumn : $sortableColum;
243246
}
244247

245248
return substr($maxColumn, 1);
@@ -265,7 +268,7 @@ public function getHighestRow($column = null)
265268
if ($c != $column) {
266269
continue;
267270
}
268-
$maxRow = max($maxRow, $r);
271+
$maxRow = ($maxRow > $r) ? $maxRow : $r;
269272
}
270273

271274
return $maxRow;

tests/PhpSpreadsheetTests/Collection/CellsTest.php

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,28 @@ public function testCollectionCell(): void
2929
self::assertSame($cell1, $collection->add('B2', $cell1), 'adding a cell should return the cell');
3030

3131
// Assert cell presence
32-
self::assertEquals(['B2'], $collection->getCoordinates(), 'cell list should contains the cell');
33-
self::assertEquals(['B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the cell');
32+
self::assertEquals(['B2'], $collection->getCoordinates(), 'cell list should contains the B2 cell');
33+
self::assertEquals(['B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the B2 cell');
3434
self::assertSame($cell1, $collection->get('B2'), 'should get exact same object');
35-
self::assertTrue($collection->has('B2'), 'cell should exists');
35+
self::assertTrue($collection->has('B2'), 'B2 cell should exists');
3636

3737
// Add a second cell
3838
$cell2 = $sheet->getCell('A1');
3939
self::assertSame($cell2, $collection->add('A1', $cell2), 'adding a second cell should return the cell');
40-
self::assertEquals(['B2', 'A1'], $collection->getCoordinates(), 'cell list should contains the cell');
41-
self::assertEquals(['A1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the cell');
40+
self::assertEquals(['B2', 'A1'], $collection->getCoordinates(), 'cell list should contains the second cell');
41+
self::assertEquals(['A1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the second cell');
42+
43+
// Add a third cell
44+
$cell3 = $sheet->getCell('AA1');
45+
self::assertSame($cell3, $collection->add('AA1', $cell3), 'adding a third cell should return the cell');
46+
self::assertEquals(['B2', 'A1', 'AA1'], $collection->getCoordinates(), 'cell list should contains the third cell');
47+
self::assertEquals(['A1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the third cell');
48+
49+
// Add a fourth cell
50+
$cell4 = $sheet->getCell('Z1');
51+
self::assertSame($cell4, $collection->add('Z1', $cell4), 'adding a fourth cell should return the cell');
52+
self::assertEquals(['B2', 'A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should contains the fourth cell');
53+
self::assertEquals(['A1', 'Z1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the fourth cell');
4254

4355
// Assert collection copy
4456
$sheet2 = $spreadsheet->createSheet();
@@ -53,16 +65,16 @@ public function testCollectionCell(): void
5365
// Assert deletion
5466
$collection->delete('B2');
5567
self::assertFalse($collection->has('B2'), 'cell should have been deleted');
56-
self::assertEquals(['A1'], $collection->getCoordinates(), 'cell list should contains the cell');
68+
self::assertEquals(['A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should still contains the A1,Z1 and A11 cells');
5769

5870
// Assert update
5971
$cell2 = $sheet->getCell('A1');
6072
self::assertSame($sheet->getCellCollection(), $collection);
6173
self::assertSame($cell2, $collection->update($cell2), 'should update existing cell');
6274

6375
$cell3 = $sheet->getCell('C3');
64-
self::assertSame($cell3, $collection->update($cell3), 'should silently add non-existing cell');
65-
self::assertEquals(['A1', 'C3'], $collection->getCoordinates(), 'cell list should contains the cell');
76+
self::assertSame($cell3, $collection->update($cell3), 'should silently add non-existing C3 cell');
77+
self::assertEquals(['A1', 'AA1', 'Z1', 'C3'], $collection->getCoordinates(), 'cell list should contains the C3 cell');
6678
}
6779

6880
public function testCacheLastCell(): void

0 commit comments

Comments
 (0)