Favor object creation in factories (#459)

Instead of passing factories in the constructors and let objects call the factory method, create all dependencies directly in the factories.

src/Spout/Reader/CSV/Creator/EntityFactory.php

@@ -37,18 +37,19 @@ class EntityFactory implements EntityFactoryInterface
      */
     public function createSheetIterator($filePointer, $optionsManager, $globalFunctionsHelper)
     {
-        return new SheetIterator($filePointer, $optionsManager, $globalFunctionsHelper, $this);
+        $rowIterator = $this->createRowIterator($filePointer, $optionsManager, $globalFunctionsHelper);
+        $sheet = $this->createSheet($rowIterator);
+
+        return new SheetIterator($sheet);
     }
 
     /**
-     * @param resource $filePointer Pointer to the CSV file to read
+     * @param RowIterator $rowIterator
-     * @param OptionsManagerInterface $optionsManager
-     * @param GlobalFunctionsHelper $globalFunctionsHelper
      * @return Sheet
      */
-    public function createSheet($filePointer, $optionsManager, $globalFunctionsHelper)
+    private function createSheet($rowIterator)
     {
-        return new Sheet($filePointer, $optionsManager, $globalFunctionsHelper, $this);
+        return new Sheet($rowIterator);
     }
 
     /**
@@ -57,7 +58,7 @@ class EntityFactory implements EntityFactoryInterface
      * @param GlobalFunctionsHelper $globalFunctionsHelper
      * @return RowIterator
      */
-    public function createRowIterator($filePointer, $optionsManager, $globalFunctionsHelper)
+    private function createRowIterator($filePointer, $optionsManager, $globalFunctionsHelper)
     {
         $encodingHelper = $this->helperFactory->createEncodingHelper($globalFunctionsHelper);
         return new RowIterator($filePointer, $optionsManager, $encodingHelper, $globalFunctionsHelper);

src/Spout/Reader/CSV/Sheet.php

@@ -16,14 +16,11 @@ class Sheet implements SheetInterface
     protected $rowIterator;
 
     /**
-     * @param resource $filePointer Pointer to the CSV file to read
+     * @param RowIterator $rowIterator Corresponding row iterator
-     * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager
-     * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
-     * @param EntityFactory $entityFactory Factory to create entities
      */
-    public function __construct($filePointer, $optionsManager, $globalFunctionsHelper, $entityFactory)
+    public function __construct(RowIterator $rowIterator)
     {
-        $this->rowIterator = $entityFactory->createRowIterator($filePointer, $optionsManager, $globalFunctionsHelper);
+        $this->rowIterator = $rowIterator;
     }
 
     /**

src/Spout/Reader/CSV/SheetIterator.php

@@ -20,14 +20,11 @@ class SheetIterator implements IteratorInterface
     protected $hasReadUniqueSheet = false;
 
     /**
-     * @param resource $filePointer
+     * @param Sheet $sheet Corresponding unique sheet
-     * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager
-     * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
-     * @param EntityFactory $entityFactory Factory to create entities
      */
-    public function __construct($filePointer, $optionsManager, $globalFunctionsHelper, $entityFactory)
+    public function __construct($sheet)
     {
-        $this->sheet = $entityFactory->createSheet($filePointer, $optionsManager, $globalFunctionsHelper);
+        $this->sheet = $sheet;
     }
 
     /**

src/Spout/Reader/ODS/Creator/EntityFactory.php

@@ -5,6 +5,7 @@ namespace Box\Spout\Reader\ODS\Creator;
 use Box\Spout\Common\Helper\GlobalFunctionsHelper;
 use Box\Spout\Common\Manager\OptionsManagerInterface;
 use Box\Spout\Reader\Common\Creator\EntityFactoryInterface;
+use Box\Spout\Reader\Common\Entity\Options;
 use Box\Spout\Reader\Common\XMLProcessor;
 use Box\Spout\Reader\ODS\RowIterator;
 use Box\Spout\Reader\ODS\Sheet;
@@ -37,7 +38,10 @@ class EntityFactory implements EntityFactoryInterface
      */
     public function createSheetIterator($filePath, $optionsManager)
     {
-        return new SheetIterator($filePath, $optionsManager, $this, $this->helperFactory);
+        $escaper = $this->helperFactory->createStringsEscaper();
+        $settingsHelper = $this->helperFactory->createSettingsHelper($this);
+
+        return new SheetIterator($filePath, $optionsManager, $escaper, $settingsHelper, $this);
     }
 
     /**
@@ -50,7 +54,9 @@ class EntityFactory implements EntityFactoryInterface
      */
     public function createSheet($xmlReader, $sheetIndex, $sheetName, $isSheetActive, $optionsManager)
     {
-        return new Sheet($xmlReader, $sheetIndex, $sheetName, $isSheetActive, $optionsManager, $this);
+        $rowIterator = $this->createRowIterator($xmlReader, $optionsManager);
+
+        return new Sheet($rowIterator, $sheetIndex, $sheetName, $isSheetActive);
     }
 
     /**
@@ -58,9 +64,13 @@ class EntityFactory implements EntityFactoryInterface
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
      * @return RowIterator
      */
-    public function createRowIterator($xmlReader, $optionsManager)
+    private function createRowIterator($xmlReader, $optionsManager)
     {
-        return new RowIterator($xmlReader, $optionsManager, $this, $this->helperFactory);
+        $shouldFormatDates = $optionsManager->getOption(Options::SHOULD_FORMAT_DATES);
+        $cellValueFormatter = $this->helperFactory->createCellValueFormatter($shouldFormatDates);
+        $xmlProcessor = $this->createXMLProcessor($xmlReader);
+
+        return new RowIterator($xmlReader, $optionsManager, $cellValueFormatter, $xmlProcessor);
     }
 
     /**
@@ -75,7 +85,7 @@ class EntityFactory implements EntityFactoryInterface
      * @param $xmlReader
      * @return XMLProcessor
      */
-    public function createXMLProcessor($xmlReader)
+    private function createXMLProcessor($xmlReader)
     {
         return new XMLProcessor($xmlReader);
     }

src/Spout/Reader/ODS/RowIterator.php

@@ -9,6 +9,7 @@ use Box\Spout\Reader\Exception\XMLProcessingException;
 use Box\Spout\Reader\IteratorInterface;
 use Box\Spout\Reader\ODS\Creator\EntityFactory;
 use Box\Spout\Reader\ODS\Creator\HelperFactory;
+use Box\Spout\Reader\ODS\Helper\CellValueFormatter;
 use Box\Spout\Reader\Wrapper\XMLReader;
 use Box\Spout\Reader\Common\XMLProcessor;
 
@@ -75,17 +76,17 @@ class RowIterator implements IteratorInterface
     /**
      * @param XMLReader $xmlReader XML Reader, positioned on the "<table:table>" element
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
-     * @param EntityFactory $entityFactory Factory to create entities
+     * @param CellValueFormatter $cellValueFormatter Helper to format cell values
-     * @param HelperFactory $helperFactory Factory to create helpers
+     * @param XMLProcessor $xmlProcessor Helper to process XML files
      */
-    public function __construct($xmlReader, $optionsManager, $entityFactory, $helperFactory)
+    public function __construct($xmlReader, $optionsManager, $cellValueFormatter, $xmlProcessor)
     {
         $this->xmlReader = $xmlReader;
         $this->shouldPreserveEmptyRows = $optionsManager->getOption(Options::SHOULD_PRESERVE_EMPTY_ROWS);
-        $this->cellValueFormatter = $helperFactory->createCellValueFormatter($optionsManager->getOption(Options::SHOULD_FORMAT_DATES));
+        $this->cellValueFormatter = $cellValueFormatter;
 
         // Register all callbacks to process different nodes when reading the XML file
-        $this->xmlProcessor = $entityFactory->createXMLProcessor($this->xmlReader);
+        $this->xmlProcessor = $xmlProcessor;
         $this->xmlProcessor->registerCallback(self::XML_NODE_ROW, XMLProcessor::NODE_TYPE_START, [$this, 'processRowStartingNode']);
         $this->xmlProcessor->registerCallback(self::XML_NODE_CELL, XMLProcessor::NODE_TYPE_START, [$this, 'processCellStartingNode']);
         $this->xmlProcessor->registerCallback(self::XML_NODE_ROW, XMLProcessor::NODE_TYPE_END, [$this, 'processRowEndingNode']);

src/Spout/Reader/ODS/Sheet.php

@@ -30,16 +30,14 @@ class Sheet implements SheetInterface
     protected $isActive;
 
     /**
-     * @param XMLReader $xmlReader XML Reader, positioned on the "<table:table>" element
+     * @param RowIterator $rowIterator The corresponding row iterator
      * @param int $sheetIndex Index of the sheet, based on order in the workbook (zero-based)
      * @param string $sheetName Name of the sheet
      * @param bool $isSheetActive Whether the sheet was defined as active
-     * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
-     * @param EntityFactory $entityFactory Factory to create entities
      */
-    public function __construct($xmlReader, $sheetIndex, $sheetName, $isSheetActive, $optionsManager, $entityFactory)
+    public function __construct($rowIterator, $sheetIndex, $sheetName, $isSheetActive)
     {
-        $this->rowIterator = $entityFactory->createRowIterator($xmlReader, $optionsManager);
+        $this->rowIterator = $rowIterator;
         $this->index = $sheetIndex;
         $this->name = $sheetName;
         $this->isActive = $isSheetActive;

src/Spout/Reader/ODS/SheetIterator.php

@@ -51,19 +51,17 @@ class SheetIterator implements IteratorInterface
     /**
      * @param string $filePath Path of the file to be read
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager
+     * @param \Box\Spout\Common\Helper\Escaper\ODS $escaper Used to unescape XML data
+     * @param SettingsHelper $settingsHelper Helper to get data from "settings.xml"
      * @param EntityFactory $entityFactory Factory to create entities
-     * @param HelperFactory $helperFactory Factory to create helpers
      */
-    public function __construct($filePath, $optionsManager, $entityFactory, $helperFactory)
+    public function __construct($filePath, $optionsManager, $escaper, $settingsHelper, $entityFactory)
     {
         $this->filePath = $filePath;
         $this->optionsManager = $optionsManager;
         $this->entityFactory = $entityFactory;
         $this->xmlReader = $entityFactory->createXMLReader();
-
+        $this->escaper = $escaper;
-        $this->escaper = $helperFactory->createStringsEscaper();
-
-        $settingsHelper = $helperFactory->createSettingsHelper($entityFactory);
         $this->activeSheetName = $settingsHelper->getActiveSheetName($filePath);
     }
 

src/Spout/Reader/XLSX/Creator/EntityFactory.php

@@ -3,6 +3,7 @@
 namespace Box\Spout\Reader\XLSX\Creator;
 
 use Box\Spout\Reader\Common\Creator\EntityFactoryInterface;
+use Box\Spout\Reader\Common\Entity\Options;
 use Box\Spout\Reader\Common\XMLProcessor;
 use Box\Spout\Reader\XLSX\Helper\SharedStringsHelper;
 use Box\Spout\Reader\XLSX\RowIterator;
@@ -33,19 +34,12 @@ class EntityFactory implements EntityFactoryInterface
      * @param string $filePath Path of the file to be read
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
      * @param SharedStringsHelper $sharedStringsHelper Helper to work with shared strings
-     * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
      * @return SheetIterator
      */
-    public function createSheetIterator($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper)
+    public function createSheetIterator($filePath, $optionsManager, $sharedStringsHelper)
     {
-        return new SheetIterator(
+        $sheetHelper = $this->helperFactory->createSheetHelper($filePath, $optionsManager, $sharedStringsHelper, $this);
-            $filePath,
+        return new SheetIterator($sheetHelper);
-            $optionsManager,
-            $sharedStringsHelper,
-            $globalFunctionsHelper,
-            $this,
-            $this->helperFactory
-        );
     }
 
     /**
@@ -67,16 +61,8 @@ class EntityFactory implements EntityFactoryInterface
         $optionsManager,
         $sharedStringsHelper)
     {
-        return new Sheet(
+        $rowIterator = $this->createRowIterator($filePath, $sheetDataXMLFilePath, $optionsManager, $sharedStringsHelper);
-            $filePath,
+        return new Sheet($rowIterator, $sheetIndex, $sheetName, $isSheetActive);
-            $sheetDataXMLFilePath,
-            $sheetIndex,
-            $sheetName,
-            $isSheetActive,
-            $optionsManager,
-            $sharedStringsHelper,
-            $this
-        );
     }
 
     /**
@@ -86,9 +72,25 @@ class EntityFactory implements EntityFactoryInterface
      * @param SharedStringsHelper $sharedStringsHelper Helper to work with shared strings
      * @return RowIterator
      */
-    public function createRowIterator($filePath, $sheetDataXMLFilePath, $optionsManager, $sharedStringsHelper)
+    private function createRowIterator($filePath, $sheetDataXMLFilePath, $optionsManager, $sharedStringsHelper)
     {
-        return new RowIterator($filePath, $sheetDataXMLFilePath, $optionsManager, $sharedStringsHelper, $this, $this->helperFactory);
+        $xmlReader = $this->createXMLReader();
+        $xmlProcessor = $this->createXMLProcessor($xmlReader);
+
+        $styleHelper = $this->helperFactory->createStyleHelper($filePath, $this);
+        $shouldFormatDates = $optionsManager->getOption(Options::SHOULD_FORMAT_DATES);
+        $cellValueFormatter = $this->helperFactory->createCellValueFormatter($sharedStringsHelper, $styleHelper, $shouldFormatDates);
+
+        $shouldPreserveEmptyRows = $optionsManager->getOption(Options::SHOULD_PRESERVE_EMPTY_ROWS);
+
+        return new RowIterator(
+            $filePath,
+            $sheetDataXMLFilePath,
+            $shouldPreserveEmptyRows,
+            $xmlReader,
+            $xmlProcessor,
+            $cellValueFormatter
+        );
     }
 
     /**

src/Spout/Reader/XLSX/Creator/HelperFactory.php

@@ -44,14 +44,13 @@ class HelperFactory extends \Box\Spout\Common\Creator\HelperFactory
      * @param string $filePath Path of the XLSX file being read
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
      * @param \Box\Spout\Reader\XLSX\Helper\SharedStringsHelper Helper to work with shared strings
-     * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
      * @param EntityFactory $entityFactory Factory to create entities
      * @return SheetHelper
      */
-    public function createSheetHelper($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $entityFactory)
+    public function createSheetHelper($filePath, $optionsManager, $sharedStringsHelper, $entityFactory)
     {
         $escaper = $this->createStringsEscaper();
-        return new SheetHelper($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $escaper, $entityFactory);
+        return new SheetHelper($filePath, $optionsManager, $sharedStringsHelper, $escaper, $entityFactory);
     }
 
     /**

src/Spout/Reader/XLSX/Helper/SheetHelper.php

@@ -53,16 +53,14 @@ class SheetHelper
      * @param string $filePath Path of the XLSX file being read
      * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
      * @param \Box\Spout\Reader\XLSX\Helper\SharedStringsHelper Helper to work with shared strings
-     * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
      * @param \Box\Spout\Common\Helper\Escaper\XLSX $escaper Used to unescape XML data
      * @param EntityFactory $entityFactory Factory to create entities
      */
-    public function __construct($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $escaper, $entityFactory)
+    public function __construct($filePath, $optionsManager, $sharedStringsHelper, $escaper, $entityFactory)
     {
         $this->filePath = $filePath;
         $this->optionsManager = $optionsManager;
         $this->sharedStringsHelper = $sharedStringsHelper;
-        $this->globalFunctionsHelper = $globalFunctionsHelper;
         $this->escaper = $escaper;
         $this->entityFactory = $entityFactory;
     }

src/Spout/Reader/XLSX/RowIterator.php

@@ -10,9 +10,8 @@ use Box\Spout\Reader\Wrapper\XMLReader;
 use Box\Spout\Reader\XLSX\Creator\EntityFactory;
 use Box\Spout\Reader\XLSX\Creator\HelperFactory;
 use Box\Spout\Reader\XLSX\Helper\CellHelper;
-use Box\Spout\Reader\XLSX\Helper\CellValueFormatter;
-use Box\Spout\Reader\XLSX\Helper\StyleHelper;
 use Box\Spout\Reader\Common\XMLProcessor;
+use Box\Spout\Reader\XLSX\Helper\CellValueFormatter;
 
 /**
  * Class RowIterator
@@ -48,9 +47,6 @@ class RowIterator implements IteratorInterface
     /** @var Helper\CellValueFormatter Helper to format cell values */
     protected $cellValueFormatter;
 
-    /** @var Helper\StyleHelper $styleHelper Helper to work with styles */
-    protected $styleHelper;
-
     /**
      * TODO: This variable can be deleted when row indices get preserved
      * @var int Number of read rows
@@ -84,25 +80,21 @@ class RowIterator implements IteratorInterface
     /**
      * @param string $filePath Path of the XLSX file being read
      * @param string $sheetDataXMLFilePath Path of the sheet data XML file as in [Content_Types].xml
-     * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
+     * @param bool $shouldPreserveEmptyRows Whether empty rows should be preserved
-     * @param Helper\SharedStringsHelper $sharedStringsHelper Helper to work with shared strings
+     * @param XMLReader $xmlReader XML Reader
-     * @param EntityFactory $entityFactory Factory to create entities
+     * @param XMLProcessor $xmlProcessor Helper to process XML files
-     * @param HelperFactory $helperFactory Factory to create helpers
+     * @param CellValueFormatter $cellValueFormatter Helper to format cell values
      */
-    public function __construct($filePath, $sheetDataXMLFilePath, $optionsManager, $sharedStringsHelper, $entityFactory, $helperFactory)
+    public function __construct($filePath, $sheetDataXMLFilePath, $shouldPreserveEmptyRows, $xmlReader, $xmlProcessor, $cellValueFormatter)
     {
         $this->filePath = $filePath;
         $this->sheetDataXMLFilePath = $this->normalizeSheetDataXMLFilePath($sheetDataXMLFilePath);
-
+        $this->xmlReader = $xmlReader;
-        $this->xmlReader = $entityFactory->createXMLReader();
+        $this->cellValueFormatter = $cellValueFormatter;
-
+        $this->shouldPreserveEmptyRows = $shouldPreserveEmptyRows;
-        $this->styleHelper = $helperFactory->createStyleHelper($filePath, $entityFactory);
-        $this->cellValueFormatter = $helperFactory->createCellValueFormatter($sharedStringsHelper, $this->styleHelper, $optionsManager->getOption(Options::SHOULD_FORMAT_DATES));
-
-        $this->shouldPreserveEmptyRows = $optionsManager->getOption(Options::SHOULD_PRESERVE_EMPTY_ROWS);
 
         // Register all callbacks to process different nodes when reading the XML file
-        $this->xmlProcessor = $entityFactory->createXMLProcessor($this->xmlReader);
+        $this->xmlProcessor = $xmlProcessor;
         $this->xmlProcessor->registerCallback(self::XML_NODE_DIMENSION, XMLProcessor::NODE_TYPE_START, [$this, 'processDimensionStartingNode']);
         $this->xmlProcessor->registerCallback(self::XML_NODE_ROW, XMLProcessor::NODE_TYPE_START, [$this, 'processRowStartingNode']);
         $this->xmlProcessor->registerCallback(self::XML_NODE_CELL, XMLProcessor::NODE_TYPE_START, [$this, 'processCellStartingNode']);

src/Spout/Reader/XLSX/Sheet.php

@@ -26,25 +26,14 @@ class Sheet implements SheetInterface
     protected $isActive;
 
     /**
-     * @param string $filePath Path of the XLSX file being read
+     * @param RowIterator $rowIterator The corresponding row iterator
-     * @param string $sheetDataXMLFilePath Path of the sheet data XML file as in [Content_Types].xml
      * @param int $sheetIndex Index of the sheet, based on order in the workbook (zero-based)
      * @param string $sheetName Name of the sheet
      * @param bool $isSheetActive Whether the sheet was defined as active
-     * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
-     * @param Helper\SharedStringsHelper $sharedStringsHelper Helper to work with shared strings
-     * @param EntityFactory $entityFactory Factory to create entities
      */
-    public function __construct(
+    public function __construct($rowIterator, $sheetIndex, $sheetName, $isSheetActive)
-        $filePath,
-        $sheetDataXMLFilePath,
-        $sheetIndex, $sheetName,
-        $isSheetActive,
-        $optionsManager,
-        $sharedStringsHelper,
-        $entityFactory)
     {
-        $this->rowIterator = $entityFactory->createRowIterator($filePath, $sheetDataXMLFilePath, $optionsManager, $sharedStringsHelper);
+        $this->rowIterator = $rowIterator;
         $this->index = $sheetIndex;
         $this->name = $sheetName;
         $this->isActive = $isSheetActive;

src/Spout/Reader/XLSX/SheetIterator.php

@@ -23,24 +23,12 @@ class SheetIterator implements IteratorInterface
     protected $currentSheetIndex;
 
     /**
-     * @param string $filePath Path of the file to be read
+     * @param SheetHelper $sheetHelper Helper to work with sheets
-     * @param \Box\Spout\Common\Manager\OptionsManagerInterface $optionsManager Reader's options manager
-     * @param \Box\Spout\Reader\XLSX\Helper\SharedStringsHelper $sharedStringsHelper
-     * @param \Box\Spout\Common\Helper\GlobalFunctionsHelper $globalFunctionsHelper
-     * @param EntityFactory $entityFactory Factory to create entities
-     * @param HelperFactory $helperFactory Factory to create helpers
      * @throws \Box\Spout\Reader\Exception\NoSheetsFoundException If there are no sheets in the file
      */
-    public function __construct(
+    public function __construct($sheetHelper)
-        $filePath,
-        $optionsManager,
-        $sharedStringsHelper,
-        $globalFunctionsHelper,
-        $entityFactory,
-        $helperFactory)
     {
         // Fetch all available sheets
-        $sheetHelper = $helperFactory->createSheetHelper($filePath, $optionsManager, $sharedStringsHelper, $globalFunctionsHelper, $entityFactory);
         $this->sheets = $sheetHelper->getSheets();
 
         if (count($this->sheets) === 0) {