Improving code quality

- remove the need for the $newline argument in createFromString #132 #130 - improve integer filtering
thephpleague · Oct 29, 2015 · 46eedd3 · 46eedd3
1 parent f67966f
commit 46eedd3
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,17 +9,17 @@ All Notable changes to `League\Csv` will be documented in this file
 - `League\Csv\Reader::fetch` replaces `League\Csv\Reader::query` for naming consistency
 - `League\Csv\Config\Controls::fetchDelimitersOccurrence` to replace `League\Csv\Config\Controls::detectDelimiterList` the latter gives erronous results
 
-
 ### Deprecated
 
 - `League\Csv\Config\Controls::detectDelimiterList`
 - `League\Csv\Reader::query`
+- The `$newline` argument from `AbstractCsv::createFromString` is deprecated
 
 ### Fixed
 
 - Bug fix Stream feature by removing trimming filter name argument [issue #122](https://github.com/thephpleague/csv/issues/122)
 
-- Bug fix SplFileObject flags usage [PR #130](https://github.com/thephpleague/csv/pull/130)
+- Bug fix `SplFileObject` flags usage [PR #130](https://github.com/thephpleague/csv/pull/130)
 
 ### Removed
 

diff --git a/src/AbstractCsv.php b/src/AbstractCsv.php
@@ -246,7 +246,7 @@ public static function createFromFileObject(SplFileObject $obj)
     public static function createFromString($str, $newline = "\n")
     {
         $file = new SplTempFileObject();
-        $file->fwrite(rtrim($str).$newline);
+        $file->fwrite($str);
 
         $obj = static::createFromFileObject($file);
         $obj->setNewline($newline);
@@ -300,4 +300,24 @@ public function newReader($open_mode = 'r+')
     {
         return $this->newInstance('\League\Csv\Reader', $open_mode);
     }
+
+    /**
+     * Validate the submitted integer
+     *
+     * @param int    $int
+     * @param int    $min
+     * @param string $message
+     *
+     * @throws InvalidArgumentException If the value is invalid
+     *
+     * @return int
+     */
+    protected function filterInteger($int, $min, $message)
+    {
+        if (false === ($int = filter_var($int, FILTER_VALIDATE_INT, ['options' => ['min_range' => $min]]))) {
+            throw new InvalidArgumentException($message);
+        }
+
+        return $int;
+    }
 }
diff --git a/src/Config/Controls.php b/src/Config/Controls.php
@@ -240,15 +240,17 @@ public function getEscape()
      */
     public function setFlags($flags)
     {
-        if (false === filter_var($flags, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]])) {
-            throw new InvalidArgumentException('you should use a `SplFileObject` Constant');
-        }
-
+        $flags = $this->filterInteger($flags, 0, 'you should use a `SplFileObject` Constant');
         $this->flags = $flags | SplFileObject::READ_CSV;
 
         return $this;
     }
 
+    /**
+     * @inheritdoc
+     */
+    abstract protected function filterInteger($int, $min, $message);
+
     /**
      * Returns the file Flags
      *

diff --git a/src/Modifier/QueryFilter.php b/src/Modifier/QueryFilter.php
@@ -14,7 +14,6 @@
 
 use ArrayObject;
 use CallbackFilterIterator;
-use InvalidArgumentException;
 use Iterator;
 use LimitIterator;
 
@@ -102,14 +101,16 @@ abstract public function getInputBom();
      */
     public function setOffset($offset = 0)
     {
-        if (false === filter_var($offset, FILTER_VALIDATE_INT, ['options' => ['min_range' => 0]])) {
-            throw new InvalidArgumentException('the offset must be a positive integer or 0');
-        }
-        $this->iterator_offset = $offset;
+        $this->iterator_offset = $this->filterInteger($offset, 0, 'the offset must be a positive integer or 0');
 
         return $this;
     }
 
+    /**
+     * @inheritdoc
+     */
+    abstract protected function filterInteger($int, $min, $message);
+
     /**
      * Set LimitIterator Count
      *
@@ -119,10 +120,7 @@ public function setOffset($offset = 0)
      */
     public function setLimit($limit = -1)
     {
-        if (false === filter_var($limit, FILTER_VALIDATE_INT, ['options' => ['min_range' => -1]])) {
-            throw new InvalidArgumentException('the limit must an integer greater or equals to -1');
-        }
-        $this->iterator_limit = $limit;
+        $this->iterator_limit = $this->filterInteger($limit, -1, 'the limit must an integer greater or equals to -1');
 
         return $this;
     }

diff --git a/test/ControlsTest.php b/test/ControlsTest.php
@@ -196,11 +196,8 @@ public function testCustomNewline()
      */
     public function testAppliedFlags($flag, $line_count)
     {
-        if (defined('HHVM_VERSION')) {
-            $this->markTestSkipped('HHVM CSV parsing is different');
-        }
         $path = __DIR__.'/data/tmp.txt';
-        $obj  = new SplFileObject($path, 'w+');
+        $obj = new SplFileObject($path, 'w+');
         $obj->fwrite("1st\n2nd\n");
         $reader = Reader::createFromFileObject($obj);
         $reader->setFlags($flag);