Merge pull request box#32 from box/rename_number_to_index

Rename *Number to *Index
ratno · Apr 29, 2015 · 52faef0 · 52faef0
2 parents c6e9430 + cfd3e0f
commit 52faef0
Show file tree

Hide file tree

Showing 12 changed files with 134 additions and 108 deletions.
diff --git a/src/Spout/Reader/AbstractReader.php b/src/Spout/Reader/AbstractReader.php
@@ -14,7 +14,7 @@
  */
 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
@@ -91,12 +91,12 @@ public function getWorksheets()
      * 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 @@ protected function getSheet($sheetDataXMLFilePath, $sheetNumberZeroBased)
             }
         }
 
-        return new Sheet($sheetId, $sheetNumberZeroBased, $sheetName);
+        return new Sheet($sheetId, $sheetIndexZeroBased, $sheetName);
     }
 
     /**
@@ -204,6 +204,6 @@ protected function getFileAsXMLElementWithNamespace($xmlFilePath, $mainNamespace
      */
     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
@@ -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 @@ public function getExternalSheet()
     /**
      * @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
@@ -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 @@ public function getId()
     }
 
     /**
-     * @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
@@ -132,8 +132,8 @@ public function nextSheet()
         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
@@ -67,8 +67,8 @@ public function __construct($tempFolder, $shouldUseInlineStrings, $shouldCreateN
      */
     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
@@ -96,8 +96,8 @@ public function getLastWrittenRowIndex()
      */
     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
@@ -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
@@ -0,0 +1,52 @@
+<?php
+
+namespace Box\Spout\Reader;
+
+use Box\Spout\Common\Type;
+use Box\Spout\TestUsingResource;
+
+/**
+ * Class SheetTest
+ *
+ * @package Box\Spout\Reader
+ */
+class SheetTest extends \PHPUnit_Framework_TestCase
+{
+    use TestUsingResource;
+
+    /**
+     * @return void
+     */
+    public function testNextSheetShouldReturnCorrectSheetInfos()
+    {
+        $sheets = $this->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
@@ -200,32 +200,6 @@ public function testReadShouldBeAbleToProcessEmptySheets()
         $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
@@ -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 @@ public function testGetSheetName()
         $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 @@ private function writeDataAndReturnSheets($fileName)
 
         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('<sheet name="' . $expectedName . '"', $xmlContents, $message);
+    }
 }