Improve internal code

3 files changed, 90 insertions, 82 deletions

diff --git a/src/AbstractCsv.php b/src/AbstractCsv.php
index e61b6e2..b776cd6 100644
--- a/src/AbstractCsv.php
+++ b/src/AbstractCsv.php
@@ -12,7 +12,6 @@
 */
 namespace League\Csv;
 
-use CallbackFilterIterator;
 use InvalidArgumentException;
 use IteratorAggregate;
 use JsonSerializable;
@@ -71,7 +70,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      *
      * can be a SplFileInfo object or the string path to a file
      *
-     * @var \SplFileObject|string
+     * @var SplFileObject|string
      */
     protected $path;
 
@@ -83,6 +82,13 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     protected $open_mode;
 
     /**
+     * Default SplFileObject flags settings
+     *
+     * @var int
+     */
+    protected $defaultFlags;
+
+    /**
      * Creates a new instance
      *
      * The path must be an SplFileInfo object
@@ -94,7 +100,8 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      */
     protected function __construct($path, $open_mode = 'r+')
     {
-        $this->flags = SplFileObject::READ_CSV | SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY;
+        $this->defaultFlags = SplFileObject::READ_CSV | SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY;
+        $this->flags = $this->defaultFlags;
         $this->open_mode = strtolower($open_mode);
         $this->path = $this->normalizePath($path);
         $this->initStreamFilter($this->path);
@@ -106,7 +113,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      *
      * @param object|string $path the filepath
      *
-     * @return \SplFileObject|string
+     * @return SplFileObject|string
      */
     protected function normalizePath($path)
     {
@@ -128,7 +135,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
     /**
      * Returns the CSV Iterator
      *
-     * @return \Iterator
+     * @return SplFileObject
      */
     public function getIterator()
     {
@@ -149,12 +156,12 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      */
     protected function getConversionIterator()
     {
-        $iterator = $this->getIterator();
-        $iterator->setFlags($this->flags | SplFileObject::READ_AHEAD | SplFileObject::SKIP_EMPTY);
-        $iterator = $this->applyBomStripping($iterator);
-        $iterator = new CallbackFilterIterator($iterator, function ($row) {
+        $this->addFilter(function ($row) {
             return is_array($row) && [null] != $row;
         });
+        $iterator = $this->getIterator();
+        $iterator->setFlags($this->defaultFlags);
+        $iterator = $this->applyBomStripping($iterator);
         $iterator = $this->applyIteratorFilter($iterator);
         $iterator = $this->applyIteratorSortBy($iterator);
 
@@ -184,7 +191,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      * @param object|string $path      file path
      * @param string        $open_mode the file open mode flag
      *
-     * @throws \InvalidArgumentException If $path is a \SplTempFileObject object
+     * @throws InvalidArgumentException If $path is a \SplTempFileObject object
      *
      * @return static
      */
@@ -275,7 +282,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      *
      * @param string $open_mode the file open mode flag
      *
-     * @return \League\Csv\Writer
+     * @return Writer
      */
     public function newWriter($open_mode = 'r+')
     {
@@ -287,7 +294,7 @@ abstract class AbstractCsv implements JsonSerializable, IteratorAggregate
      *
      * @param string $open_mode the file open mode flag
      *
-     * @return \League\Csv\Reader
+     * @return Reader
      */
     public function newReader($open_mode = 'r+')
     {
diff --git a/src/Config/Controls.php b/src/Config/Controls.php
index 99149b7..6e6de06 100644
--- a/src/Config/Controls.php
+++ b/src/Config/Controls.php
@@ -72,7 +72,7 @@ trait Controls
      */
     public function setDelimiter($delimiter)
     {
-        if (1 != mb_strlen($delimiter)) {
+        if (!$this->isValidCsvControls($delimiter)) {
             throw new InvalidArgumentException('The delimiter must be a single character');
         }
         $this->delimiter = $delimiter;
@@ -81,6 +81,18 @@ trait Controls
     }
 
     /**
+     * Tell whether the submitted string is a valid CSV Control character
+     *
+     * @param strung $str The submitted string
+     *
+     * @return bool
+     */
+    protected function isValidCsvControls($str)
+    {
+        return 1 == mb_strlen($str);
+    }
+
+    /**
      * Returns the current field delimiter
      *
      * @return string
@@ -132,48 +144,30 @@ trait Controls
      */
     public function fetchDelimitersOccurrence(array $delimiters, $nb_rows = 1)
     {
-        $nb_rows = filter_var($nb_rows, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]);
-        if (!$nb_rows) {
+        if (!($nb_rows = filter_var($nb_rows, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]))) {
             throw new InvalidArgumentException('The number of rows to consider must be a valid positive integer');
         }
-        $delimiters = array_filter($delimiters, function ($str) {
-            return 1 == mb_strlen($str);
-        });
-        $delimiters = array_unique($delimiters);
+
+        $filterRow = function ($row) {
+            return is_array($row) && count($row) > 1;
+        };
+        $delimiters = array_unique(array_filter($delimiters, [$this, 'isValidCsvControls']));
+        $csv = $this->getIterator();
         $res = [];
         foreach ($delimiters as $delim) {
-            $res[$delim] = $this->fetchRowsCountByDelimiter($delim, $nb_rows);
+            $csv->setCsvControl($delim, $this->enclosure, $this->escape);
+            $iterator = new CallbackFilterIterator(new LimitIterator($csv, 0, $nb_rows), $filterRow);
+            $res[$delim] = count(iterator_to_array($iterator, false), COUNT_RECURSIVE);
         }
-
         arsort($res, SORT_NUMERIC);
 
         return $res;
     }
 
     /**
-     * Detects the actual number of row according to a delimiter
-     *
-     * @param string $delimiter a CSV delimiter
-     * @param int    $nb_rows   the number of row to consider
-     *
-     * @return int
-     */
-    protected function fetchRowsCountByDelimiter($delimiter, $nb_rows = 1)
-    {
-        $iterator = $this->getIterator();
-        $iterator->setCsvControl($delimiter, $this->enclosure, $this->escape);
-        $iterator = new LimitIterator($iterator, 0, $nb_rows);
-        $iterator = new CallbackFilterIterator($iterator, function ($row) {
-            return is_array($row) && count($row) > 1;
-        });
-
-        return count(iterator_to_array($iterator, false), COUNT_RECURSIVE);
-    }
-
-    /**
      * Returns the CSV Iterator
      *
-     * @return \Iterator
+     * @return SplFileObject
      */
     abstract public function getIterator();
 
@@ -188,7 +182,7 @@ trait Controls
      */
     public function setEnclosure($enclosure)
     {
-        if (1 != mb_strlen($enclosure)) {
+        if (!$this->isValidCsvControls($enclosure)) {
             throw new InvalidArgumentException('The enclosure must be a single character');
         }
         $this->enclosure = $enclosure;
@@ -217,7 +211,7 @@ trait Controls
      */
     public function setEscape($escape)
     {
-        if (1 != mb_strlen($escape)) {
+        if (!$this->isValidCsvControls($escape)) {
             throw new InvalidArgumentException('The escape character must be a single character');
         }
         $this->escape = $escape;
diff --git a/src/Reader.php b/src/Reader.php
index 7523c9f..9da44a6 100644
--- a/src/Reader.php
+++ b/src/Reader.php
@@ -56,12 +56,11 @@ class Reader extends AbstractCsv
      */
     public function fetch(callable $callable = null)
     {
-        $filterArray = function ($row) {
+        $this->addFilter(function ($row) {
             return is_array($row);
-        };
+        });
         $iterator = $this->getIterator();
         $iterator = $this->applyBomStripping($iterator);
-        $iterator = new CallbackFilterIterator($iterator, $filterArray);
         $iterator = $this->applyIteratorFilter($iterator);
         $iterator = $this->applyIteratorSortBy($iterator);
         $iterator = $this->applyIteratorInterval($iterator);
@@ -178,20 +177,13 @@ class Reader extends AbstractCsv
      *
      * @throws InvalidArgumentException If the submitted keys are invalid
      *
-     * @return array
+     * @return Iterator
      */
     public function fetchAssoc($offset_or_keys = 0, callable $callable = null)
     {
         $keys = $this->getAssocKeys($offset_or_keys);
-        $this->assertValidAssocKeys($keys);
-        if (!is_array($offset_or_keys)) {
-            $filterOutOffset = function ($row, $rowIndex) use ($offset_or_keys) {
-                return is_array($row) && $rowIndex != $offset_or_keys;
-            };
-            $this->addFilter($filterOutOffset);
-        }
-        $combineArray = function (array $row) use ($keys) {
-            $keys_count = count($keys);
+        $keys_count = count($keys);
+        $combineArray = function (array $row) use ($keys, $keys_count) {
             if ($keys_count != count($row)) {
                 $row = array_slice(array_pad($row, $keys_count, null), 0, $keys_count);
             }
@@ -205,7 +197,7 @@ class Reader extends AbstractCsv
     /**
      * Selects the array to be used as key for the fetchAssoc method
      *
-     * @param array|int $offset_or_keys the assoc key OR the row Index to be used
+     * @param int|array $offset_or_keys the assoc key OR the row Index to be used
      *                                  as the key index
      *
      * @throws InvalidArgumentException If the row index and/or the resulting array is invalid
@@ -214,17 +206,50 @@ class Reader extends AbstractCsv
      */
     protected function getAssocKeys($offset_or_keys)
     {
-        if (is_array($offset_or_keys) && ! empty($offset_or_keys)) {
+        if (is_array($offset_or_keys)) {
+            $this->assertValidAssocKeys($offset_or_keys);
+
             return $offset_or_keys;
         }
 
         if (false === filter_var($offset_or_keys, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]])) {
-            throw new InvalidArgumentException(
-                'the row index must be a positive integer, 0 or a non empty array'
-            );
+            throw new InvalidArgumentException('the row index must be a positive integer, 0 or a non empty array');
         }
 
-        return $this->getRow($offset_or_keys);
+        $keys = $this->getRow($offset_or_keys);
+        $this->assertValidAssocKeys($keys);
+        $filterOutRow = function ($row, $rowIndex) use ($offset_or_keys) {
+            return is_array($row) && $rowIndex != $offset_or_keys;
+        };
+        $this->addFilter($filterOutRow);
+
+        return $keys;
+    }
+
+    /**
+     * Validates the array to be used by the fetchAssoc method
+     *
+     * @param array $keys
+     *
+     * @throws InvalidArgumentException If the submitted array fails the assertion
+     */
+    protected function assertValidAssocKeys(array $keys)
+    {
+        if (empty($keys) || $keys !== array_unique(array_filter($keys, [$this, 'isValidString']))) {
+            throw new InvalidArgumentException('Use a flat array with unique string values');
+        }
+    }
+
+    /**
+     * Returns whether the submitted value can be used as string
+     *
+     * @param mixed $value
+     *
+     * @return bool
+     */
+    protected function isValidString($value)
+    {
+        return is_scalar($value) || (is_object($value) && method_exists($value, '__toString'));
     }
 
     /**
@@ -240,10 +265,10 @@ class Reader extends AbstractCsv
     {
         $csv = $this->getIterator();
         $csv->setFlags($this->getFlags() & ~SplFileObject::READ_CSV);
-
         $iterator = new LimitIterator($csv, $offset, 1);
         $iterator->rewind();
         $res = $iterator->current();
+
         if (empty($res)) {
             throw new InvalidArgumentException('the specified row does not exist or is empty');
         }
@@ -254,22 +279,4 @@ class Reader extends AbstractCsv
 
         return str_getcsv($res, $this->delimiter, $this->enclosure, $this->escape);
     }
-
-    /**
-     * Validates the array to be used by the fetchAssoc method
-     *
-     * @param array $keys
-     *
-     * @throws InvalidArgumentException If the submitted array fails the assertion
-     */
-    protected function assertValidAssocKeys(array $keys)
-    {
-        $test = array_unique(array_filter($keys, function ($value) {
-            return is_scalar($value) || (is_object($value) && method_exists($value, '__toString'));
-        }));
-
-        if ($keys !== $test) {
-            throw new InvalidArgumentException('Use a flat array with unique string values');
-        }
-    }
 }