Improve v8 code quality

- Improve Reader::fetchPairs when the offsetIndex or the valueIndex are undefined
- Internal refactoring

7 files changed, 139 insertions, 156 deletions

diff --git a/src/AbstractCsv.php b/src/AbstractCsv.php
index 5ab72a1..799666b 100644
--- a/src/AbstractCsv.php
+++ b/src/AbstractCsv.php
@@ -118,79 +118,33 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     }
 
     /**
-     * Returns the inner SplFileObject
+     * Return a new {@link AbstractCsv} from a SplFileObject
      *
-     * @return SplFileObject
-     */
-    public function getIterator()
-    {
-        $this->returnType = self::TYPE_ARRAY;
-        $iterator = $this->path;
-        if (!$iterator instanceof SplFileObject) {
-            $iterator = new SplFileObject($this->getStreamFilterPath(), $this->open_mode);
-        }
-        $iterator->setCsvControl($this->delimiter, $this->enclosure, $this->escape);
-        $iterator->setFlags(SplFileObject::READ_CSV | SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY);
-
-        return $iterator;
-    }
-
-    /**
-     * Returns the CSV Iterator
+     * @param SplFileObject $file
      *
-     * @return Iterator
+     * @return static
      */
-    protected function getCsvIterator()
+    public static function createFromFileObject(SplFileObject $file)
     {
-        array_unshift($this->iterator_filters, function ($row) {
-            return is_array($row) && $row != [null];
-        });
-        $iterator = $this->getIterator();
-        $iterator = $this->applyBomStripping($iterator);
-        $iterator = $this->applyIteratorFilter($iterator);
-        $iterator = $this->applyIteratorSortBy($iterator);
-
-        return $this->applyIteratorInterval($iterator);
+        return new static($file);
     }
 
     /**
-     * Creates a {@link AbstractCsv} from a string
-     *
-     * The path can be:
-     * - an SplFileInfo,
-     * - a SplFileObject,
-     * - an object that implements the `__toString` method,
-     * - a string
+     * Return a new {@link AbstractCsv} from a string
      *
-     * BUT NOT a SplTempFileObject
-     *
-     * <code>
-     *<?php
-     * $csv = new Reader::createFromPath('/path/to/file.csv', 'a+');
-     * $csv = new Reader::createFromPath(new SplFileInfo('/path/to/file.csv'));
-     * $csv = new Reader::createFromPath(new SplFileObject('/path/to/file.csv'), 'rb');
-     *
-     * ?>
-     * </code>
-     *
-     * @param mixed  $path      file path
-     * @param string $open_mode the file open mode flag
+     * The string must be an object that implements the `__toString` method,
+     * or a string
      *
-     * @throws InvalidArgumentException If $path is a SplTempFileObject object
+     * @param string|object $str the string
      *
      * @return static
      */
-    public static function createFromPath($path, $open_mode = 'r+')
+    public static function createFromString($str)
     {
-        if ($path instanceof SplTempFileObject) {
-            throw new InvalidArgumentException('an `SplTempFileObject` object does not contain a valid path');
-        }
-
-        if ($path instanceof SplFileInfo) {
-            $path = $path->getPath().'/'.$path->getBasename();
-        }
+        $file = new SplTempFileObject();
+        $file->fwrite(static::validateString($str));
 
-        return new static(static::validateString($path), $open_mode);
+        return new static($file);
     }
 
     /**
@@ -211,49 +165,30 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     }
 
     /**
-     * Creates a {@link AbstractCsv} from a SplFileObject
-     *
-     * The path can be:
-     * - a SplFileObject,
-     * - a SplTempFileObject
-     *
-     * <code>
-     *<?php
-     * $csv = new Writer::createFromFileObject(new SplFileInfo('/path/to/file.csv'));
-     * $csv = new Writer::createFromFileObject(new SplTempFileObject);
+     * Return a new {@link AbstractCsv} from a string
      *
-     * ?>
-     * </code>
+     * @param mixed  $path      file path
+     * @param string $open_mode the file open mode flag
      *
-     * @param SplFileObject $file
+     * @throws InvalidArgumentException If $path is a SplTempFileObject object
      *
      * @return static
      */
-    public static function createFromFileObject(SplFileObject $file)
+    public static function createFromPath($path, $open_mode = 'r+')
     {
-        return new static($file);
-    }
+        if ($path instanceof SplTempFileObject) {
+            throw new InvalidArgumentException('an `SplTempFileObject` object does not contain a valid path');
+        }
 
-    /**
-     * Creates a {@link AbstractCsv} from a string
-     *
-     * The string must be an object that implements the `__toString` method,
-     * or a string
-     *
-     * @param string|object $str the string
-     *
-     * @return static
-     */
-    public static function createFromString($str)
-    {
-        $file = new SplTempFileObject();
-        $file->fwrite(static::validateString($str));
+        if ($path instanceof SplFileInfo) {
+            $path = $path->getPath().'/'.$path->getBasename();
+        }
 
-        return static::createFromFileObject($file);
+        return new static(static::validateString($path), $open_mode);
     }
 
     /**
-     * Creates a {@link AbstractCsv} instance from another {@link AbstractCsv} object
+     * Return a new {@link AbstractCsv} instance from another {@link AbstractCsv} object
      *
      * @param string $class     the class to be instantiated
      * @param string $open_mode the file open mode flag
@@ -275,7 +210,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     }
 
     /**
-     * Creates a {@link Writer} instance from a {@link AbstractCsv} object
+     * Return a new {@link Writer} instance from a {@link AbstractCsv} object
      *
      * @param string $open_mode the file open mode flag
      *
@@ -287,7 +222,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     }
 
     /**
-     * Creates a {@link Reader} instance from a {@link AbstractCsv} object
+     * Return a new {@link Reader} instance from a {@link AbstractCsv} object
      *
      * @param string $open_mode the file open mode flag
      *
@@ -299,21 +234,19 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     }
 
     /**
-     * Validate the submitted integer
-     *
-     * @param int    $int
-     * @param int    $minValue
-     * @param string $errorMessage
-     *
-     * @throws InvalidArgumentException If the value is invalid
+     * Returns the inner SplFileObject
      *
-     * @return int
+     * @return SplFileObject
      */
-    protected function filterInteger($int, $minValue, $errorMessage)
+    public function getIterator()
     {
-        if (false === ($int = filter_var($int, FILTER_VALIDATE_INT, ['options' => ['min_range' => $minValue]]))) {
-            throw new InvalidArgumentException($errorMessage);
+        $iterator = $this->path;
+        if (!$iterator instanceof SplFileObject) {
+            $iterator = new SplFileObject($this->getStreamFilterPath(), $this->open_mode);
         }
-        return $int;
+        $iterator->setCsvControl($this->delimiter, $this->enclosure, $this->escape);
+        $iterator->setFlags(SplFileObject::READ_CSV | SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY);
+
+        return $iterator;
     }
 }
diff --git a/src/Config/Controls.php b/src/Config/Controls.php
index 195af6f..bf1166e 100644
--- a/src/Config/Controls.php
+++ b/src/Config/Controls.php
@@ -108,7 +108,7 @@ trait Controls
      */
     public function fetchDelimitersOccurrence(array $delimiters, $nb_rows = 1)
     {
-        $nb_rows = $this->filterInteger($nb_rows, 1, 'The number of rows to consider must be a valid positive integer');
+        $nb_rows = $this->validateInteger($nb_rows, 1, 'The number of rows to consider must be a valid positive integer');
         $filterRow = function ($row) {
             return is_array($row) && count($row) > 1;
         };
@@ -126,9 +126,23 @@ trait Controls
     }
 
     /**
-     * @inheritdoc
+     * Validate an integer
+     *
+     * @param int    $int
+     * @param int    $minValue
+     * @param string $errorMessage
+     *
+     * @throws InvalidArgumentException If the value is invalid
+     *
+     * @return int
      */
-    abstract protected function filterInteger($int, $minValue, $errorMessage);
+    protected function validateInteger($int, $minValue, $errorMessage)
+    {
+        if (false === ($int = filter_var($int, FILTER_VALIDATE_INT, ['options' => ['min_range' => $minValue]]))) {
+            throw new InvalidArgumentException($errorMessage);
+        }
+        return $int;
+    }
 
     /**
      * Returns the CSV Iterator
diff --git a/src/Config/Output.php b/src/Config/Output.php
index b1ee6ef..8ae235e 100644
--- a/src/Config/Output.php
+++ b/src/Config/Output.php
@@ -113,7 +113,7 @@ trait Output
      */
     public function getInputBOM()
     {
-        if (! $this->input_bom) {
+        if (!$this->input_bom) {
             $bom = [
                 AbstractCsv::BOM_UTF32_BE, AbstractCsv::BOM_UTF32_LE,
                 AbstractCsv::BOM_UTF16_BE, AbstractCsv::BOM_UTF16_LE, AbstractCsv::BOM_UTF8,
@@ -202,7 +202,7 @@ trait Output
      */
     public function jsonSerialize()
     {
-        return iterator_to_array($this->convertToUtf8($this->getCsvIterator()), false);
+        return iterator_to_array($this->convertToUtf8($this->getQueryIterator()), false);
     }
 
     /**
@@ -210,7 +210,7 @@ trait Output
      *
      * @return Iterator
      */
-    abstract protected function getCsvIterator();
+    abstract protected function getQueryIterator();
 
     /**
      * Convert Csv file into UTF-8
@@ -264,7 +264,7 @@ trait Output
     {
         $doc = new DomDocument('1.0', 'UTF-8');
         $root = $doc->createElement($root_name);
-        $iterator = $this->convertToUtf8($this->getCsvIterator());
+        $iterator = $this->convertToUtf8($this->getQueryIterator());
         foreach ($iterator as $row) {
             $item = $doc->createElement($row_name);
             array_walk($row, function ($value) use (&$item, $doc, $cell_name) {
diff --git a/src/Modifier/QueryFilter.php b/src/Modifier/QueryFilter.php
index 176517b..50519b2 100644
--- a/src/Modifier/QueryFilter.php
+++ b/src/Modifier/QueryFilter.php
@@ -113,7 +113,7 @@ trait QueryFilter
     {
         $bom = $this->getInputBom();
 
-        return ! empty($bom) && $this->strip_bom;
+        return !empty($bom) && $this->strip_bom;
     }
 
     /**
@@ -130,7 +130,7 @@ trait QueryFilter
      */
     public function setOffset($offset = 0)
     {
-        $this->iterator_offset = $this->filterInteger($offset, 0, 'the offset must be a positive integer or 0');
+        $this->iterator_offset = $this->validateInteger($offset, 0, 'the offset must be a positive integer or 0');
 
         return $this;
     }
@@ -138,7 +138,7 @@ trait QueryFilter
     /**
      * @inheritdoc
      */
-    abstract protected function filterInteger($int, $minValue, $errorMessage);
+    abstract protected function validateInteger($int, $minValue, $errorMessage);
 
     /**
      * Set LimitIterator Count
@@ -149,7 +149,7 @@ trait QueryFilter
      */
     public function setLimit($limit = -1)
     {
-        $this->iterator_limit = $this->filterInteger($limit, -1, 'the limit must an integer greater or equals to -1');
+        $this->iterator_limit = $this->validateInteger($limit, -1, 'the limit must an integer greater or equals to -1');
 
         return $this;
     }
@@ -269,11 +269,11 @@ trait QueryFilter
      */
     protected function applyBomStripping(Iterator $iterator)
     {
-        if (! $this->strip_bom) {
+        if (!$this->strip_bom) {
             return $iterator;
         }
 
-        if (! $this->isBomStrippable()) {
+        if (!$this->isBomStrippable()) {
             $this->strip_bom = false;
 
             return $iterator;
@@ -315,6 +315,30 @@ trait QueryFilter
     abstract public function getEnclosure();
 
     /**
+     * Returns the CSV Iterator
+     *
+     * @return Iterator
+     */
+    protected function getQueryIterator()
+    {
+        array_unshift($this->iterator_filters, function ($row) {
+            return is_array($row) && $row != [null];
+        });
+        $iterator = $this->getIterator();
+        $iterator = $this->applyBomStripping($iterator);
+        $iterator = $this->applyIteratorFilter($iterator);
+        $iterator = $this->applyIteratorSortBy($iterator);
+        $this->returnType = AbstractCsv::TYPE_ARRAY;
+
+        return $this->applyIteratorInterval($iterator);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    abstract public function getIterator();
+
+    /**
     * Filter the Iterator
     *
     * @param Iterator $iterator
@@ -360,7 +384,7 @@ trait QueryFilter
     */
     protected function applyIteratorSortBy(Iterator $iterator)
     {
-        if (! $this->iterator_sort_by) {
+        if (!$this->iterator_sort_by) {
             return $iterator;
         }
         $obj = new ArrayObject(iterator_to_array($iterator, false));
diff --git a/src/Modifier/StreamFilter.php b/src/Modifier/StreamFilter.php
index caf0c76..030523a 100644
--- a/src/Modifier/StreamFilter.php
+++ b/src/Modifier/StreamFilter.php
@@ -208,10 +208,16 @@ trait StreamFilter
     protected function sanitizeStreamFilter($filter_name)
     {
         $this->assertStreamable();
-        return (string) $filter_name;
+
+        return $this->validateString($filter_name);
     }
 
     /**
+     * @inheritdoc
+     */
+    abstract public function validateString($str);
+
+    /**
      * Detect if the stream filter is already present
      *
      * @param string $filter_name
@@ -264,7 +270,7 @@ trait StreamFilter
     protected function getStreamFilterPath()
     {
         $this->assertStreamable();
-        if (! $this->stream_filters) {
+        if (!$this->stream_filters) {
             return $this->stream_uri;
         }
 
diff --git a/src/Reader.php b/src/Reader.php
index 4aad520..3f6d84d 100644
--- a/src/Reader.php
+++ b/src/Reader.php
@@ -42,7 +42,7 @@ class Reader extends AbstractCsv
      */
     public function fetch(callable $callable = null)
     {
-        return $this->applyCallable($this->getCsvIterator(), $callable);
+        return $this->applyCallable($this->getQueryIterator(), $callable);
     }
 
     /**
@@ -137,8 +137,7 @@ class Reader extends AbstractCsv
      */
     public function fetchColumn($columnIndex = 0, callable $callable = null)
     {
-        $type = $this->returnType;
-        $columnIndex = $this->filterInteger($columnIndex, 0, 'the column index must be a positive integer or 0');
+        $columnIndex = $this->validateInteger($columnIndex, 0, 'the column index must be a positive integer or 0');
 
         $filterColumn = function ($row) use ($columnIndex) {
             return isset($row[$columnIndex]);
@@ -149,10 +148,11 @@ class Reader extends AbstractCsv
         };
 
         $this->addFilter($filterColumn);
+        $returnType = $this->returnType;
         $iterator = $this->fetch($selectColumn);
         $iterator = $this->applyCallable($iterator, $callable);
 
-        return $this->applyReturnType($type, $iterator, false);
+        return $this->applyReturnType($returnType, $iterator, false);
     }
 
     /**
@@ -165,28 +165,32 @@ class Reader extends AbstractCsv
      * - the first CSV column is used to provide the keys
      * - the second CSV column is used to provide the value
      *
-     * @param int           $offsetColumnIndex The column index to server as offset
-     * @param int           $valueColumnIndex  The column index to server as value
-     * @param callable|null $callable          A callable to be applied to each of the rows to be returned.
+     * @param int           $offsetIndex The column index to serve as offset
+     * @param int           $valueIndex  The column index to serve as value
+     * @param callable|null $callable    A callable to be applied to each of the rows to be returned.
      *
      * @return Generator|array
      */
-    public function fetchPairs($offsetColumnIndex = 0, $valueColumnIndex = 1, callable $callable = null)
+    public function fetchPairs($offsetIndex = 0, $valueIndex = 1, callable $callable = null)
     {
-        $type = $this->returnType;
-        $offsetColumnIndex = $this->filterInteger($offsetColumnIndex, 0, 'the offset column index must be a positive integer or 0');
-        $valueColumnIndex = $this->filterInteger($valueColumnIndex, 0, 'the value column index must be a positive integer or 0');
-        $filterPairs = function ($row) use ($offsetColumnIndex, $valueColumnIndex) {
-            return isset($row[$offsetColumnIndex], $row[$valueColumnIndex]);
+        $offsetIndex = $this->validateInteger($offsetIndex, 0, 'the offset column index must be a positive integer or 0');
+        $valueIndex = $this->validateInteger($valueIndex, 0, 'the value column index must be a positive integer or 0');
+        $filterPairs = function ($row) use ($offsetIndex, $valueIndex) {
+            return isset($row[$offsetIndex]);
         };
-        $selectPairs = function ($row) use ($offsetColumnIndex, $valueColumnIndex) {
-            return [$row[$offsetColumnIndex], $row[$valueColumnIndex]];
+        $selectPairs = function ($row) use ($offsetIndex, $valueIndex) {
+            return [
+                $row[$offsetIndex],
+                isset($row[$valueIndex]) ? $row[$valueIndex] : null,
+            ];
         };
+
         $this->addFilter($filterPairs);
+        $returnType = $this->returnType;
         $iterator = $this->fetch($selectPairs);
         $iterator = $this->applyCallable($iterator, $callable);
 
-        return $this->applyReturnType($type, $this->fetchPairsGenerator($iterator));
+        return $this->applyReturnType($returnType, $this->generatePairs($iterator));
     }
 
     /**
@@ -196,7 +200,7 @@ class Reader extends AbstractCsv
      *
      * @return Generator
      */
-    protected function fetchPairsGenerator(Iterator $iterator)
+    protected function generatePairs(Iterator $iterator)
     {
         foreach ($iterator as $row) {
             yield $row[0] => $row[1];
@@ -220,7 +224,6 @@ class Reader extends AbstractCsv
      */
     public function fetchAssoc($offset_or_keys = 0, callable $callable = null)
     {
-        $type = $this->returnType;
         $keys = $this->getAssocKeys($offset_or_keys);
         $keys_count = count($keys);
         $combineArray = function (array $row) use ($keys, $keys_count) {
@@ -230,10 +233,12 @@ class Reader extends AbstractCsv
 
             return array_combine($keys, $row);
         };
+
+        $returnType = $this->returnType;
         $iterator = $this->fetch($combineArray);
         $iterator = $this->applyCallable($iterator, $callable);
 
-        return $this->applyReturnType($type, $iterator, false);
+        return $this->applyReturnType($returnType, $iterator, false);
     }
 
     /**
@@ -249,18 +254,15 @@ class Reader extends AbstractCsv
     protected function getAssocKeys($offset_or_keys)
     {
         if (is_array($offset_or_keys)) {
-            $this->assertValidAssocKeys($offset_or_keys);
-
-            return $offset_or_keys;
+            return $this->validateKeys($offset_or_keys);
         }
 
-        $offset_or_keys = $this->filterInteger(
+        $offset_or_keys = $this->validateInteger(
             $offset_or_keys,
             0,
             'the row index must be a positive integer, 0 or a non empty array'
         );
-        $keys = $this->getRow($offset_or_keys);
-        $this->assertValidAssocKeys($keys);
+        $keys = $this->validateKeys($this->getRow($offset_or_keys));
         $filterOutRow = function ($row, $rowIndex) use ($offset_or_keys) {
             return $rowIndex != $offset_or_keys;
         };
@@ -275,12 +277,16 @@ class Reader extends AbstractCsv
      * @param array $keys
      *
      * @throws InvalidArgumentException If the submitted array fails the assertion
+     *
+     * @return array
      */
-    protected function assertValidAssocKeys(array $keys)
+    protected function validateKeys(array $keys)
     {
         if (empty($keys) || $keys !== array_unique(array_filter($keys, [$this, 'isValidKey']))) {
             throw new InvalidArgumentException('Use a flat array with unique string values');
         }
+
+        return $keys;
     }
 
     /**
@@ -306,20 +312,20 @@ class Reader extends AbstractCsv
      */
     protected function getRow($offset)
     {
-        $iterator = $this->getIterator();
-        $iterator->setFlags(SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY);
-        $iterator = new LimitIterator($iterator, $offset, 1);
+        $fileObj = $this->getIterator();
+        $fileObj->setFlags(SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY);
+        $iterator = new LimitIterator($fileObj, $offset, 1);
         $iterator->rewind();
-        $res = $iterator->current();
+        $line = $iterator->current();
 
-        if (empty($res)) {
+        if (empty($line)) {
             throw new InvalidArgumentException('the specified row does not exist or is empty');
         }
 
-        if (0 == $offset && $this->isBomStrippable()) {
-            $res = mb_substr($res, mb_strlen($this->getInputBom()));
+        if (0 === $offset && $this->isBomStrippable()) {
+            $line = mb_substr($line, mb_strlen($this->getInputBom()));
         }
 
-        return str_getcsv($res, $this->delimiter, $this->enclosure, $this->escape);
+        return str_getcsv($line, $this->delimiter, $this->enclosure, $this->escape);
     }
 }
diff --git a/src/Writer.php b/src/Writer.php
index 8fd94f8..a35af37 100644
--- a/src/Writer.php
+++ b/src/Writer.php
@@ -70,7 +70,7 @@ class Writer extends AbstractCsv
     protected static function initFputcsv()
     {
         if (null === static::$fputcsv) {
-            static::$fputcsv  = new ReflectionMethod('\SplFileObject', 'fputcsv');
+            static::$fputcsv = new ReflectionMethod('\SplFileObject', 'fputcsv');
             static::$fputcsv_param_count = static::$fputcsv->getNumberOfParameters();
         }
     }