Skip to content

Commit 5e090d1

Browse files
dkarloviPowerKiKi
authored andcommitted
Allow iterators to go out of bounds with prev()
Iterators prev() behavior is now consistent with next(), meaning that it can go out of bounds and it must be validated with valid() before using it. Fixes #587 Fixes #627
1 parent 125f462 commit 5e090d1

File tree

11 files changed

+44
-32
lines changed

11 files changed

+44
-32
lines changed

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](http://keepachangelog.com/)
66
and this project adheres to [Semantic Versioning](http://semver.org/).
77

8+
## [Unreleased]
9+
10+
### Fixed
11+
12+
- Allow iterators to go out of bounds with prev - [#587](https://github.com/PHPOffice/PhpSpreadsheet/issues/587)
13+
814
## [1.4.0] - 2018-08-06
915

1016
### Added

src/PhpSpreadsheet/Worksheet/ColumnCellIterator.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,10 +153,6 @@ public function next()
153153
*/
154154
public function prev()
155155
{
156-
if ($this->currentRow <= $this->startRow) {
157-
throw new PhpSpreadsheetException("Row is already at the beginning of range ({$this->startRow} - {$this->endRow})");
158-
}
159-
160156
do {
161157
--$this->currentRow;
162158
} while (($this->onlyExistingCells) &&
@@ -171,7 +167,7 @@ public function prev()
171167
*/
172168
public function valid()
173169
{
174-
return $this->currentRow <= $this->endRow;
170+
return $this->currentRow <= $this->endRow && $this->currentRow >= $this->startRow;
175171
}
176172

177173
/**

src/PhpSpreadsheet/Worksheet/ColumnIterator.php

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,9 @@ public function next()
157157

158158
/**
159159
* Set the iterator to its previous value.
160-
*
161-
* @throws PhpSpreadsheetException
162160
*/
163161
public function prev()
164162
{
165-
if ($this->currentColumnIndex <= $this->startColumnIndex) {
166-
throw new PhpSpreadsheetException('Column is already at the beginning of range (' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ' - ' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ')');
167-
}
168163
--$this->currentColumnIndex;
169164
}
170165

@@ -175,6 +170,6 @@ public function prev()
175170
*/
176171
public function valid()
177172
{
178-
return $this->currentColumnIndex <= $this->endColumnIndex;
173+
return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex;
179174
}
180175
}

src/PhpSpreadsheet/Worksheet/Iterator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class Iterator implements \Iterator
2525
*
2626
* @param Spreadsheet $subject
2727
*/
28-
public function __construct(Spreadsheet $subject = null)
28+
public function __construct(Spreadsheet $subject)
2929
{
3030
// Set subject
3131
$this->subject = $subject;
@@ -82,6 +82,6 @@ public function next()
8282
*/
8383
public function valid()
8484
{
85-
return $this->position < $this->subject->getSheetCount();
85+
return $this->position < $this->subject->getSheetCount() && $this->position >= 0;
8686
}
8787
}

src/PhpSpreadsheet/Worksheet/RowCellIterator.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,6 @@ public function next()
155155
*/
156156
public function prev()
157157
{
158-
if ($this->currentColumnIndex <= $this->startColumnIndex) {
159-
throw new PhpSpreadsheetException('Column is already at the beginning of range (' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ' - ' . Coordinate::stringFromColumnIndex($this->endColumnIndex) . ')');
160-
}
161158
do {
162159
--$this->currentColumnIndex;
163160
} while (($this->onlyExistingCells) && (!$this->worksheet->cellExistsByColumnAndRow($this->currentColumnIndex, $this->rowIndex)) && ($this->currentColumnIndex >= $this->startColumnIndex));
@@ -170,7 +167,7 @@ public function prev()
170167
*/
171168
public function valid()
172169
{
173-
return $this->currentColumnIndex <= $this->endColumnIndex;
170+
return $this->currentColumnIndex <= $this->endColumnIndex && $this->currentColumnIndex >= $this->startColumnIndex;
174171
}
175172

176173
/**

src/PhpSpreadsheet/Worksheet/RowIterator.php

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -152,15 +152,9 @@ public function next()
152152

153153
/**
154154
* Set the iterator to its previous value.
155-
*
156-
* @throws PhpSpreadsheetException
157155
*/
158156
public function prev()
159157
{
160-
if ($this->position <= $this->startRow) {
161-
throw new PhpSpreadsheetException("Row is already at the beginning of range ({$this->startRow} - {$this->endRow})");
162-
}
163-
164158
--$this->position;
165159
}
166160

@@ -171,6 +165,6 @@ public function prev()
171165
*/
172166
public function valid()
173167
{
174-
return $this->position <= $this->endRow;
168+
return $this->position <= $this->endRow && $this->position >= $this->startRow;
175169
}
176170
}

tests/PhpSpreadsheetTests/Worksheet/ColumnCellIteratorTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,8 @@ public function testSeekOutOfRange()
7878

7979
public function testPrevOutOfRange()
8080
{
81-
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
82-
8381
$iterator = new ColumnCellIterator($this->mockWorksheet, 'A', 2, 4);
8482
$iterator->prev();
83+
self::assertFalse($iterator->valid());
8584
}
8685
}

tests/PhpSpreadsheetTests/Worksheet/ColumnIteratorTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,8 @@ public function testSeekOutOfRange()
7777

7878
public function testPrevOutOfRange()
7979
{
80-
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
81-
8280
$iterator = new ColumnIterator($this->mockWorksheet, 'B', 'D');
8381
$iterator->prev();
82+
self::assertFalse($iterator->valid());
8483
}
8584
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
4+
5+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
6+
use PhpOffice\PhpSpreadsheet\Worksheet\Iterator;
7+
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
8+
use PHPUnit\Framework\TestCase;
9+
10+
class IteratorTest extends TestCase
11+
{
12+
public function testIteratorFullRange()
13+
{
14+
$spreadsheet = new Spreadsheet();
15+
$spreadsheet->createSheet();
16+
$spreadsheet->createSheet();
17+
18+
$iterator = new Iterator($spreadsheet);
19+
$columnIndexResult = 0;
20+
self::assertEquals($columnIndexResult, $iterator->key());
21+
22+
foreach ($iterator as $key => $column) {
23+
self::assertEquals($columnIndexResult++, $key);
24+
self::assertInstanceOf(Worksheet::class, $column);
25+
}
26+
self::assertSame(3, $columnIndexResult);
27+
}
28+
}

tests/PhpSpreadsheetTests/Worksheet/RowCellIteratorTest.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,8 @@ public function testSeekOutOfRange()
8080

8181
public function testPrevOutOfRange()
8282
{
83-
$this->expectException(\PhpOffice\PhpSpreadsheet\Exception::class);
84-
8583
$iterator = new RowCellIterator($this->mockWorksheet, 2, 'B', 'D');
8684
$iterator->prev();
85+
self::assertFalse($iterator->valid());
8786
}
8887
}

0 commit comments

Comments
 (0)