From cfd3e0ffa3a00a3bb6cec286649ecba657588c68 Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Wed, 29 Apr 2015 10:48:31 -0700 Subject: [PATCH] Rename *Number to *Index --- src/Spout/Reader/AbstractReader.php | 2 +- .../Reader/Helper/XLSX/WorksheetHelper.php | 10 ++-- src/Spout/Reader/Internal/XLSX/Worksheet.php | 14 ++--- src/Spout/Reader/Sheet.php | 16 +++--- src/Spout/Reader/XLSX.php | 4 +- src/Spout/Writer/Internal/XLSX/Workbook.php | 4 +- src/Spout/Writer/Internal/XLSX/Worksheet.php | 4 +- src/Spout/Writer/Sheet.php | 18 +++---- tests/Spout/Reader/SheetTest.php | 52 +++++++++++++++++++ tests/Spout/Reader/XLSXTest.php | 26 ---------- tests/Spout/Writer/SheetTest.php | 44 ++++++++++++++-- tests/Spout/Writer/XLSXTest.php | 48 +++-------------- 12 files changed, 134 insertions(+), 108 deletions(-) create mode 100644 tests/Spout/Reader/SheetTest.php diff --git a/src/Spout/Reader/AbstractReader.php b/src/Spout/Reader/AbstractReader.php index f2d6a05..2e2e2de 100644 --- a/src/Spout/Reader/AbstractReader.php +++ b/src/Spout/Reader/AbstractReader.php @@ -14,7 +14,7 @@ use Box\Spout\Reader\Exception\EndOfFileReachedException; */ abstract class AbstractReader implements ReaderInterface { - /** @var int Used to keep track of the row number */ + /** @var int Used to keep track of the row index */ protected $currentRowIndex = 0; /** @var bool Indicates whether the stream is currently open */ diff --git a/src/Spout/Reader/Helper/XLSX/WorksheetHelper.php b/src/Spout/Reader/Helper/XLSX/WorksheetHelper.php index f531b37..d869dd4 100644 --- a/src/Spout/Reader/Helper/XLSX/WorksheetHelper.php +++ b/src/Spout/Reader/Helper/XLSX/WorksheetHelper.php @@ -91,12 +91,12 @@ class WorksheetHelper * default to the data sheet XML file name ("xl/worksheets/sheet2.xml" => "sheet2"). * * @param string $sheetDataXMLFilePath Path of the sheet data XML file as in [Content_Types].xml - * @param int $sheetNumberZeroBased Index of the sheet, based on order in [Content_Types].xml (zero-based) + * @param int $sheetIndexZeroBased Index of the sheet, based on order in [Content_Types].xml (zero-based) * @return \Box\Spout\Reader\Sheet Sheet instance */ - protected function getSheet($sheetDataXMLFilePath, $sheetNumberZeroBased) + protected function getSheet($sheetDataXMLFilePath, $sheetIndexZeroBased) { - $sheetId = $sheetNumberZeroBased + 1; + $sheetId = $sheetIndexZeroBased + 1; $sheetName = $this->getDefaultSheetName($sheetDataXMLFilePath); /* @@ -126,7 +126,7 @@ class WorksheetHelper } } - return new Sheet($sheetId, $sheetNumberZeroBased, $sheetName); + return new Sheet($sheetId, $sheetIndexZeroBased, $sheetName); } /** @@ -204,6 +204,6 @@ class WorksheetHelper */ public function hasNextWorksheet($currentWorksheet, $allWorksheets) { - return ($currentWorksheet === null || ($currentWorksheet->getWorksheetNumber() + 1 < count($allWorksheets))); + return ($currentWorksheet === null || ($currentWorksheet->getWorksheetIndex() + 1 < count($allWorksheets))); } } diff --git a/src/Spout/Reader/Internal/XLSX/Worksheet.php b/src/Spout/Reader/Internal/XLSX/Worksheet.php index 4fd6ca2..bcf6d2f 100644 --- a/src/Spout/Reader/Internal/XLSX/Worksheet.php +++ b/src/Spout/Reader/Internal/XLSX/Worksheet.php @@ -13,21 +13,21 @@ class Worksheet /** @var \Box\Spout\Reader\Sheet The "external" sheet */ protected $externalSheet; - /** @var int Worksheet number, based on the order of appareance in [Content_Types].xml (zero-based) */ - protected $worksheetNumber; + /** @var int Worksheet index, based on the order of appareance in [Content_Types].xml (zero-based) */ + protected $worksheetIndex; /** @var string Path of the XML file containing the worksheet data */ protected $dataXmlFilePath; /**\ * @param \Box\Spout\Reader\Sheet $externalSheet The associated "external" sheet - * @param int $worksheetNumber Worksheet number, based on the order of appareance in [Content_Types].xml (zero-based) + * @param int $worksheetIndex Worksheet index, based on the order of appareance in [Content_Types].xml (zero-based) * @param string $dataXmlFilePath Path of the XML file containing the worksheet data */ - public function __construct($externalSheet, $worksheetNumber, $dataXmlFilePath) + public function __construct($externalSheet, $worksheetIndex, $dataXmlFilePath) { $this->externalSheet = $externalSheet; - $this->worksheetNumber = $worksheetNumber; + $this->worksheetIndex = $worksheetIndex; $this->dataXmlFilePath = $dataXmlFilePath; } @@ -50,8 +50,8 @@ class Worksheet /** * @return int */ - public function getWorksheetNumber() + public function getWorksheetIndex() { - return $this->worksheetNumber; + return $this->worksheetIndex; } } diff --git a/src/Spout/Reader/Sheet.php b/src/Spout/Reader/Sheet.php index 5b6b0ab..c8603ad 100644 --- a/src/Spout/Reader/Sheet.php +++ b/src/Spout/Reader/Sheet.php @@ -13,21 +13,21 @@ class Sheet /** @var int ID of the sheet */ protected $id; - /** @var int Number of the sheet, based on order of creation (zero-based) */ - protected $number; + /** @var int Index of the sheet, based on order of creation (zero-based) */ + protected $index; /** @var string Name of the sheet */ protected $name; /** * @param int $sheetId ID of the sheet - * @param int $sheetNumber Number of the sheet, based on order of creation (zero-based) + * @param int $sheetIndex Index of the sheet, based on order of creation (zero-based) * @param string $sheetName Name of the sheet */ - function __construct($sheetId, $sheetNumber, $sheetName) + function __construct($sheetId, $sheetIndex, $sheetName) { $this->id = $sheetId; - $this->number = $sheetNumber; + $this->index = $sheetIndex; $this->name = $sheetName; } @@ -40,11 +40,11 @@ class Sheet } /** - * @return int Number of the sheet, based on order of creation (zero-based) + * @return int Index of the sheet, based on order of creation (zero-based) */ - public function getNumber() + public function getIndex() { - return $this->number; + return $this->index; } /** diff --git a/src/Spout/Reader/XLSX.php b/src/Spout/Reader/XLSX.php index 2fa85f0..a95be81 100644 --- a/src/Spout/Reader/XLSX.php +++ b/src/Spout/Reader/XLSX.php @@ -132,8 +132,8 @@ class XLSX extends AbstractReader if ($this->currentWorksheet === null) { $nextWorksheet = $this->worksheets[0]; } else { - $currentWorksheetNumber = $this->currentWorksheet->getWorksheetNumber(); - $nextWorksheet = $this->worksheets[$currentWorksheetNumber + 1]; + $currentWorksheetIndex = $this->currentWorksheet->getWorksheetIndex(); + $nextWorksheet = $this->worksheets[$currentWorksheetIndex + 1]; } $this->initXmlReaderForWorksheetData($nextWorksheet); diff --git a/src/Spout/Writer/Internal/XLSX/Workbook.php b/src/Spout/Writer/Internal/XLSX/Workbook.php index e6ca0db..569d27f 100644 --- a/src/Spout/Writer/Internal/XLSX/Workbook.php +++ b/src/Spout/Writer/Internal/XLSX/Workbook.php @@ -67,8 +67,8 @@ class Workbook */ public function addNewSheet() { - $newSheetNumber = count($this->worksheets); - $sheet = new Sheet($newSheetNumber); + $newSheetIndex = count($this->worksheets); + $sheet = new Sheet($newSheetIndex); $worksheetFilesFolder = $this->fileSystemHelper->getXlWorksheetsFolder(); $worksheet = new Worksheet($sheet, $worksheetFilesFolder, $this->sharedStringsHelper, $this->shouldUseInlineStrings); diff --git a/src/Spout/Writer/Internal/XLSX/Worksheet.php b/src/Spout/Writer/Internal/XLSX/Worksheet.php index c94ec59..2dd83c3 100644 --- a/src/Spout/Writer/Internal/XLSX/Worksheet.php +++ b/src/Spout/Writer/Internal/XLSX/Worksheet.php @@ -96,8 +96,8 @@ EOD; */ public function getId() { - // sheet number is zero-based, while ID is 1-based - return $this->externalSheet->getNumber() + 1; + // sheet index is zero-based, while ID is 1-based + return $this->externalSheet->getIndex() + 1; } /** diff --git a/src/Spout/Writer/Sheet.php b/src/Spout/Writer/Sheet.php index f077028..7f8a874 100644 --- a/src/Spout/Writer/Sheet.php +++ b/src/Spout/Writer/Sheet.php @@ -12,27 +12,27 @@ class Sheet { const DEFAULT_SHEET_NAME_PREFIX = 'Sheet'; - /** @var int Number of the sheet, based on order of creation (zero-based) */ - protected $sheetNumber; + /** @var int Index of the sheet, based on order of creation (zero-based) */ + protected $index; /** @var string Name of the sheet */ protected $name; /** - * @param $sheetNumber Number of the sheet, based on order of creation (zero-based) + * @param int $sheetIndex Index of the sheet, based on order of creation (zero-based) */ - function __construct($sheetNumber) + function __construct($sheetIndex) { - $this->sheetNumber = $sheetNumber; - $this->name = self::DEFAULT_SHEET_NAME_PREFIX . ($sheetNumber + 1); + $this->index = $sheetIndex; + $this->name = self::DEFAULT_SHEET_NAME_PREFIX . ($sheetIndex + 1); } /** - * @return int Number of the sheet, based on order of creation (zero-based) + * @return int Index of the sheet, based on order of creation (zero-based) */ - public function getNumber() + public function getIndex() { - return $this->sheetNumber; + return $this->index; } /** diff --git a/tests/Spout/Reader/SheetTest.php b/tests/Spout/Reader/SheetTest.php new file mode 100644 index 0000000..5f6e02d --- /dev/null +++ b/tests/Spout/Reader/SheetTest.php @@ -0,0 +1,52 @@ +openFileAndReturnSheets('two_sheets_with_custom_names.xlsx'); + + $this->assertEquals('CustomName1', $sheets[0]->getName()); + $this->assertEquals(0, $sheets[0]->getIndex()); + $this->assertEquals(1, $sheets[0]->getId()); + + $this->assertEquals('CustomName2', $sheets[1]->getName()); + $this->assertEquals(1, $sheets[1]->getIndex()); + $this->assertEquals(2, $sheets[1]->getId()); + } + + /** + * @param string $fileName + * @return Sheet[] + */ + private function openFileAndReturnSheets($fileName) + { + $resourcePath = $this->getResourcePath($fileName); + $reader = ReaderFactory::create(Type::XLSX); + $reader->open($resourcePath); + + $sheets = []; + while ($reader->hasNextSheet()) { + $sheets[] = $reader->nextSheet(); + } + + $reader->close(); + + return $sheets; + } +} diff --git a/tests/Spout/Reader/XLSXTest.php b/tests/Spout/Reader/XLSXTest.php index 3a1a34f..23d26bc 100644 --- a/tests/Spout/Reader/XLSXTest.php +++ b/tests/Spout/Reader/XLSXTest.php @@ -200,32 +200,6 @@ class XLSXTest extends \PHPUnit_Framework_TestCase $this->assertEquals([], $allRows, 'Sheet with no cells should be correctly processed.'); } - /** - * @return void - */ - public function testNextSheetShouldReturnCorrectSheetInfos() - { - $resourcePath = $this->getResourcePath('two_sheets_with_custom_names.xlsx'); - $reader = ReaderFactory::create(Type::XLSX); - $reader->open($resourcePath); - - /** @var \Box\Spout\Reader\Sheet[] $sheets */ - $sheets = []; - while ($reader->hasNextSheet()) { - $sheets[] = $reader->nextSheet(); - } - - $reader->close(); - - $this->assertEquals('CustomName1', $sheets[0]->getName()); - $this->assertEquals(0, $sheets[0]->getNumber()); - $this->assertEquals(1, $sheets[0]->getId()); - - $this->assertEquals('CustomName2', $sheets[1]->getName()); - $this->assertEquals(1, $sheets[1]->getNumber()); - $this->assertEquals(2, $sheets[1]->getId()); - } - /** * @param string $fileName * @return array All the read rows the given file diff --git a/tests/Spout/Writer/SheetTest.php b/tests/Spout/Writer/SheetTest.php index a79e803..c9a3e55 100644 --- a/tests/Spout/Writer/SheetTest.php +++ b/tests/Spout/Writer/SheetTest.php @@ -17,13 +17,13 @@ class SheetTest extends \PHPUnit_Framework_TestCase /** * @return void */ - public function testGetSheetNumber() + public function testGetSheetIndex() { - $sheets = $this->writeDataAndReturnSheets('test_get_sheet_number.xlsx'); + $sheets = $this->writeDataAndReturnSheets('test_get_sheet_index.xlsx'); $this->assertEquals(2, count($sheets), '2 sheets should have been created'); - $this->assertEquals(0, $sheets[0]->getNumber(), 'The first sheet should be number 0'); - $this->assertEquals(1, $sheets[1]->getNumber(), 'The second sheet should be number 1'); + $this->assertEquals(0, $sheets[0]->getIndex(), 'The first sheet should be index 0'); + $this->assertEquals(1, $sheets[1]->getIndex(), 'The second sheet should be index 1'); } /** @@ -38,6 +38,28 @@ class SheetTest extends \PHPUnit_Framework_TestCase $this->assertEquals('Sheet2', $sheets[1]->getName(), 'Invalid name for the second sheet'); } + /** + * @return void + */ + public function testSetSheetNameShouldCreateSheetWithCustomName() + { + $fileName = 'test_set_name_should_create_sheet_with_custom_name.xlsx'; + $this->createGeneratedFolderIfNeeded($fileName); + $resourcePath = $this->getGeneratedResourcePath($fileName); + + $writer = WriterFactory::create(Type::XLSX); + $writer->openToFile($resourcePath); + + $customSheetName = 'CustomName'; + $sheet = $writer->getCurrentSheet(); + $sheet->setName($customSheetName); + + $writer->addRow(['xlsx--11', 'xlsx--12']); + $writer->close(); + + $this->assertSheetNameEquals($customSheetName, $resourcePath, "The sheet name should have been changed to '$customSheetName'"); + } + /** * @param string $fileName * @return Sheet[] @@ -59,4 +81,18 @@ class SheetTest extends \PHPUnit_Framework_TestCase return $writer->getSheets(); } + + /** + * @param string $expectedName + * @param string $resourcePath + * @param string $message + * @return void + */ + private function assertSheetNameEquals($expectedName, $resourcePath, $message = '') + { + $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; + $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); + + $this->assertContains('assertInlineDataWasWrittenToSheet($fileName, 1, 'control's _x0015_ "character"'); } - /** - * @return void - */ - public function testSetNameShouldCreateSheetWithCustomName() - { - $fileName = 'test_set_name_should_create_sheet_with_custom_name.xlsx'; - $this->createGeneratedFolderIfNeeded($fileName); - $resourcePath = $this->getGeneratedResourcePath($fileName); - - $writer = WriterFactory::create(Type::XLSX); - $writer->openToFile($resourcePath); - - $customSheetName = 'CustomName'; - $sheet = $writer->getCurrentSheet(); - $sheet->setName($customSheetName); - - $writer->addRow(['xlsx--11', 'xlsx--12']); - $writer->close(); - - $this->assertSheetNameEquals($customSheetName, $resourcePath, "The sheet name should have been changed to '$customSheetName'"); - } - /** * @param array $allRows * @param string $fileName @@ -422,15 +400,15 @@ class XLSXTest extends \PHPUnit_Framework_TestCase /** * @param string $fileName - * @param int $sheetNumber + * @param int $sheetIndex * @param mixed $inlineData * @param string $message * @return void */ - private function assertInlineDataWasWrittenToSheet($fileName, $sheetNumber, $inlineData, $message = '') + private function assertInlineDataWasWrittenToSheet($fileName, $sheetIndex, $inlineData, $message = '') { $resourcePath = $this->getGeneratedResourcePath($fileName); - $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetNumber . '.xml'; + $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml'; $xmlContents = file_get_contents('zip://' . $pathToSheetFile); $this->assertContains((string)$inlineData, $xmlContents, $message); @@ -438,15 +416,15 @@ class XLSXTest extends \PHPUnit_Framework_TestCase /** * @param string $fileName - * @param int $sheetNumber + * @param int $sheetIndex * @param mixed $inlineData * @param string $message * @return void */ - private function assertInlineDataWasNotWrittenToSheet($fileName, $sheetNumber, $inlineData, $message = '') + private function assertInlineDataWasNotWrittenToSheet($fileName, $sheetIndex, $inlineData, $message = '') { $resourcePath = $this->getGeneratedResourcePath($fileName); - $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetNumber . '.xml'; + $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml'; $xmlContents = file_get_contents('zip://' . $pathToSheetFile); $this->assertNotContains((string)$inlineData, $xmlContents, $message); @@ -466,18 +444,4 @@ class XLSXTest extends \PHPUnit_Framework_TestCase $this->assertContains($sharedString, $xmlContents, $message); } - - /** - * @param string $expectedName - * @param string $resourcePath - * @param string $message - * @return void - */ - private function assertSheetNameEquals($expectedName, $resourcePath, $message = '') - { - $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; - $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - - $this->assertContains('