From 4013843edcadd0ad1d9494030e2261888bfb06b5 Mon Sep 17 00:00:00 2001 From: Robert Churchill Date: Tue, 3 Nov 2015 16:27:40 +0000 Subject: [PATCH 1/4] Implement chunking without use of group iterator --- .travis.yml | 2 - README.md | 4 +- composer.json | 5 +- src/Fusonic/Linq/Iterator/ChunkIterator.php | 98 +++++++++++++++++++ src/Fusonic/Linq/Linq.php | 25 +---- tests/Fusonic/Linq/Test/ChunkIteratorTest.php | 72 ++++++++++++++ tests/Fusonic/Linq/Test/LinqTest.php | 22 +++++ 7 files changed, 201 insertions(+), 27 deletions(-) create mode 100644 src/Fusonic/Linq/Iterator/ChunkIterator.php create mode 100644 tests/Fusonic/Linq/Test/ChunkIteratorTest.php diff --git a/.travis.yml b/.travis.yml index 7dd41a7..ee8eed4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,5 @@ language: php php: - - 5.3 - - 5.4 - 5.5 - 5.6 install: composer install diff --git a/README.md b/README.md index 1ea2b38..87e6b48 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ LINQ queries offer three main advantages over traditional foreach loops: ## Requirements -fusonic/linq is supported on PHP 5.3 and up. +fusonic/linq is supported on PHP 5.5 and up. ## Installation & Usage @@ -202,6 +202,6 @@ Linq::from(array(1, 2, "Not a numeric value")) You can run the test suite with the following command: ```bash -phpunit --bootstrap tests/bootstrap.php . +vendor/bin/phpunit --bootstrap tests/bootstrap.php . ``` diff --git a/composer.json b/composer.json index 0b7e84e..69ab508 100644 --- a/composer.json +++ b/composer.json @@ -12,11 +12,14 @@ } ], "require": { - "php": ">=5.3.2" + "php": ">=5.5.0" }, "autoload": { "psr-0": { "Fusonic\\Linq": "src/" } + }, + "require-dev": { + "phpunit/phpunit": "^5.0" } } diff --git a/src/Fusonic/Linq/Iterator/ChunkIterator.php b/src/Fusonic/Linq/Iterator/ChunkIterator.php new file mode 100644 index 0000000..b327447 --- /dev/null +++ b/src/Fusonic/Linq/Iterator/ChunkIterator.php @@ -0,0 +1,98 @@ +iterator = $iterator; + $this->chunkSize = $chunkSize; + } + + /** + * @return Linq + * @todo Should this chunking logic stay here? This might be better in `next()` but then `current()` should return + * the first chunk before we call `next()`. With current implementation, looping through without calling + * `current()` is inconsistent, hence the `count()` implementation. + */ + public function current() + { + $chunk = []; + while ($this->valid()) { + $chunk[] = $this->iterator->current(); + + if (count($chunk) < $this->chunkSize) { + $this->iterator->next(); + } else { + break; + } + } + + return new Linq($chunk); + } + + public function next() + { + $this->iterator->next(); + $this->i++; + } + + public function key() + { + return $this->i; + } + + public function valid() + { + return $this->iterator->valid(); + } + + public function rewind() + { + $this->iterator->rewind(); + $this->i = 0; + } + + /** + * Implemented to ensure tests pass. + * + * @return int + */ + public function count() + { + return (int) ceil(iterator_count($this->iterator) / $this->chunkSize); + } +} diff --git a/src/Fusonic/Linq/Linq.php b/src/Fusonic/Linq/Linq.php index f9a5681..aa955e8 100644 --- a/src/Fusonic/Linq/Linq.php +++ b/src/Fusonic/Linq/Linq.php @@ -12,6 +12,7 @@ namespace Fusonic\Linq; use Countable; +use Fusonic\Linq\Iterator\ChunkIterator; use Fusonic\Linq\Iterator\ExceptIterator; use Fusonic\Linq\Iterator\DistinctIterator; use Fusonic\Linq\Iterator\GroupIterator; @@ -176,7 +177,7 @@ public function aggregate($func, $seed = null) /** * Splits the sequence in chunks according to $chunksize. * - * @param $chunksize Specifies how many elements are grouped together per chunk. + * @param int $chunksize Specifies how many elements are grouped together per chunk. * @throws \InvalidArgumentException * @return Linq */ @@ -186,27 +187,7 @@ public function chunk($chunksize) throw new \InvalidArgumentException("chunksize", $chunksize); } - $i = -1; - return $this->select( - function ($x) use (&$i) { - $i++; - return array("index" => $i, "value" => $x); - } - ) - ->groupBy( - function ($pair) use ($chunksize) { - return $pair["index"] / $chunksize; - } - ) - ->select( - function (GroupedLinq $group) { - return $group->select( - function ($v) { - return $v["value"]; - } - ); - } - ); + return Linq::from(new ChunkIterator($this->iterator, $chunksize)); } /** diff --git a/tests/Fusonic/Linq/Test/ChunkIteratorTest.php b/tests/Fusonic/Linq/Test/ChunkIteratorTest.php new file mode 100644 index 0000000..9755b94 --- /dev/null +++ b/tests/Fusonic/Linq/Test/ChunkIteratorTest.php @@ -0,0 +1,72 @@ +assertEquals(false, $x->valid()); + } + + public function testChunkSizeLargerThanIterator() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1]), 100); + + $this->assertEquals(true, $x->valid()); + $this->assertEquals([0, 1], $x->current()->toArray()); + } + + public function testChunkSizeSmallerThanIterator() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + + $this->assertEquals(true, $x->valid()); + $this->assertEquals([0, 1], $x->current()->toArray()); + + $x->next(); + $this->assertEquals(true, $x->valid()); + $this->assertEquals([2], $x->current()->toArray()); + + $this->assertEquals(false, $x->valid()); + } + + public function testCharElementsScenario() + { + $x = new ChunkIterator(new \ArrayIterator(["a", "b", "c", "d", "e"]), 2); + + $this->assertEquals(true, $x->valid()); + $this->assertEquals(["a", "b"], $x->current()->toArray()); + + $x->next(); + $this->assertEquals(true, $x->valid()); + $this->assertEquals(["c", "d"], $x->current()->toArray()); + + $x->next(); + $this->assertEquals(true, $x->valid()); + $this->assertEquals(["e"], $x->current()->toArray()); + + $this->assertEquals(false, $x->valid()); + } + + public function testDifferentChunkSize() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1, 2, 3, 4, 5, 6, 7, 8]), 3); + + $arr = array_map( + function (Linq $y) { + return implode(",", $y->toArray()); + }, + iterator_to_array($x) + ); + $this->assertEquals( + ["0,1,2", "3,4,5", "6,7,8"], + $arr + ); + } +} diff --git a/tests/Fusonic/Linq/Test/LinqTest.php b/tests/Fusonic/Linq/Test/LinqTest.php index 86fafc9..6763cc1 100644 --- a/tests/Fusonic/Linq/Test/LinqTest.php +++ b/tests/Fusonic/Linq/Test/LinqTest.php @@ -1698,6 +1698,28 @@ public function testIssue3_emtpyCollectionOrdering() ->toArray(); } + public function testChunkWithoutGrouping() + { + $log = []; + Linq::from([0, 1, 2, 3]) + ->where(function($x) use(&$log) { + $log[] = 'where'; + return true; + }) + ->select(function($x) use(&$log) { + $log[] = 'select'; + return $x; + }) + ->chunk(2) + ->each(function(Linq $chunk) use(&$log) { + $log[] = 'each'; + return $chunk; + }) + ; + + $this->assertEquals('where select where select each where select where select each', implode(' ', $log)); + } + /** * @test */ From 1eebe80ad023d155d4d303ddcc88831da60d234f Mon Sep 17 00:00:00 2001 From: Robert Churchill Date: Tue, 3 Nov 2015 16:45:51 +0000 Subject: [PATCH 2/4] Fix for php vs phpunit version conflict --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 69ab508..6242c4c 100644 --- a/composer.json +++ b/composer.json @@ -20,6 +20,6 @@ } }, "require-dev": { - "phpunit/phpunit": "^5.0" + "phpunit/phpunit": "^4.0" } } From e726798205bb291a33f9aa7f5d9e6a3228b0462f Mon Sep 17 00:00:00 2001 From: Robert Churchill Date: Wed, 4 Nov 2015 14:59:01 +0000 Subject: [PATCH 3/4] More correct implementation of iterator --- src/Fusonic/Linq/Iterator/ChunkIterator.php | 48 +++++++++---------- tests/Fusonic/Linq/Test/ChunkIteratorTest.php | 43 +++++++++++++++++ 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/Fusonic/Linq/Iterator/ChunkIterator.php b/src/Fusonic/Linq/Iterator/ChunkIterator.php index b327447..034c70d 100644 --- a/src/Fusonic/Linq/Iterator/ChunkIterator.php +++ b/src/Fusonic/Linq/Iterator/ChunkIterator.php @@ -12,20 +12,24 @@ namespace Fusonic\Linq\Iterator; -use Countable; use Fusonic\Linq\Linq; use Iterator; /** * Iterates over an iterator, returning Linq objects of the given chunk size. */ -class ChunkIterator implements Iterator, Countable +class ChunkIterator implements Iterator { /** * @var Iterator */ private $iterator; + /** + * @var array + */ + private $chunk; + /** * @var int */ @@ -40,18 +44,29 @@ public function __construct(Iterator $iterator, $chunkSize) { $this->iterator = $iterator; $this->chunkSize = $chunkSize; + + $this->chunk = $this->getNextChunk(); } /** * @return Linq - * @todo Should this chunking logic stay here? This might be better in `next()` but then `current()` should return - * the first chunk before we call `next()`. With current implementation, looping through without calling - * `current()` is inconsistent, hence the `count()` implementation. */ public function current() + { + return new Linq($this->chunk); + } + + public function next() + { + $this->iterator->next(); + $this->chunk = $this->getNextChunk(); + $this->i++; + } + + private function getNextChunk() { $chunk = []; - while ($this->valid()) { + while ($this->iterator->valid()) { $chunk[] = $this->iterator->current(); if (count($chunk) < $this->chunkSize) { @@ -61,13 +76,7 @@ public function current() } } - return new Linq($chunk); - } - - public function next() - { - $this->iterator->next(); - $this->i++; + return $chunk; } public function key() @@ -77,22 +86,13 @@ public function key() public function valid() { - return $this->iterator->valid(); + return !empty($this->chunk); } public function rewind() { $this->iterator->rewind(); $this->i = 0; - } - - /** - * Implemented to ensure tests pass. - * - * @return int - */ - public function count() - { - return (int) ceil(iterator_count($this->iterator) / $this->chunkSize); + $this->chunk = $this->getNextChunk(); } } diff --git a/tests/Fusonic/Linq/Test/ChunkIteratorTest.php b/tests/Fusonic/Linq/Test/ChunkIteratorTest.php index 9755b94..4f1999b 100644 --- a/tests/Fusonic/Linq/Test/ChunkIteratorTest.php +++ b/tests/Fusonic/Linq/Test/ChunkIteratorTest.php @@ -33,6 +33,7 @@ public function testChunkSizeSmallerThanIterator() $this->assertEquals(true, $x->valid()); $this->assertEquals([2], $x->current()->toArray()); + $x->next(); $this->assertEquals(false, $x->valid()); } @@ -51,6 +52,7 @@ public function testCharElementsScenario() $this->assertEquals(true, $x->valid()); $this->assertEquals(["e"], $x->current()->toArray()); + $x->next(); $this->assertEquals(false, $x->valid()); } @@ -69,4 +71,45 @@ function (Linq $y) { $arr ); } + + public function testIdempotency() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + + $this->assertEquals(true, $x->valid()); + $this->assertEquals([0, 1], $x->current()->toArray()); + $this->assertEquals([0, 1], $x->current()->toArray()); + $x->next(); + + $this->assertEquals([2], $x->current()->toArray()); + $this->assertEquals([2], $x->current()->toArray()); + } + + public function testNext() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + + $x->next(); + $this->assertEquals([2], $x->current()->toArray()); + } + + public function testKey() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + + $this->assertEquals(0, $x->key()); + $x->next(); + $this->assertEquals(1, $x->key()); + } + + public function testRewind() + { + $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + + $x->next(); + $x->rewind(); + $this->assertEquals(true, $x->valid()); + $this->assertEquals(0, $x->key()); + $this->assertEquals([0, 1], $x->current()->toArray()); + } } From de4f192ec452029dd08ad0038099db3bab0ac7fc Mon Sep 17 00:00:00 2001 From: Robert Churchill Date: Wed, 4 Nov 2015 15:19:34 +0000 Subject: [PATCH 4/4] Rewind must always be called first in iterator --- src/Fusonic/Linq/Iterator/ChunkIterator.php | 2 -- tests/Fusonic/Linq/Test/ChunkIteratorTest.php | 8 ++++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Fusonic/Linq/Iterator/ChunkIterator.php b/src/Fusonic/Linq/Iterator/ChunkIterator.php index 034c70d..f34d5e8 100644 --- a/src/Fusonic/Linq/Iterator/ChunkIterator.php +++ b/src/Fusonic/Linq/Iterator/ChunkIterator.php @@ -44,8 +44,6 @@ public function __construct(Iterator $iterator, $chunkSize) { $this->iterator = $iterator; $this->chunkSize = $chunkSize; - - $this->chunk = $this->getNextChunk(); } /** diff --git a/tests/Fusonic/Linq/Test/ChunkIteratorTest.php b/tests/Fusonic/Linq/Test/ChunkIteratorTest.php index 4f1999b..b0d044e 100644 --- a/tests/Fusonic/Linq/Test/ChunkIteratorTest.php +++ b/tests/Fusonic/Linq/Test/ChunkIteratorTest.php @@ -11,6 +11,7 @@ public function testEmptyIterator() { $x = new ChunkIterator(new \ArrayIterator([]), 1); + $x->rewind(); $this->assertEquals(false, $x->valid()); } @@ -18,6 +19,7 @@ public function testChunkSizeLargerThanIterator() { $x = new ChunkIterator(new \ArrayIterator([0, 1]), 100); + $x->rewind(); $this->assertEquals(true, $x->valid()); $this->assertEquals([0, 1], $x->current()->toArray()); } @@ -26,6 +28,7 @@ public function testChunkSizeSmallerThanIterator() { $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + $x->rewind(); $this->assertEquals(true, $x->valid()); $this->assertEquals([0, 1], $x->current()->toArray()); @@ -41,6 +44,7 @@ public function testCharElementsScenario() { $x = new ChunkIterator(new \ArrayIterator(["a", "b", "c", "d", "e"]), 2); + $x->rewind(); $this->assertEquals(true, $x->valid()); $this->assertEquals(["a", "b"], $x->current()->toArray()); @@ -76,6 +80,7 @@ public function testIdempotency() { $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + $x->rewind(); $this->assertEquals(true, $x->valid()); $this->assertEquals([0, 1], $x->current()->toArray()); $this->assertEquals([0, 1], $x->current()->toArray()); @@ -89,6 +94,7 @@ public function testNext() { $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + $x->rewind(); $x->next(); $this->assertEquals([2], $x->current()->toArray()); } @@ -97,6 +103,7 @@ public function testKey() { $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + $x->rewind(); $this->assertEquals(0, $x->key()); $x->next(); $this->assertEquals(1, $x->key()); @@ -106,6 +113,7 @@ public function testRewind() { $x = new ChunkIterator(new \ArrayIterator([0, 1, 2]), 2); + $x->rewind(); $x->next(); $x->rewind(); $this->assertEquals(true, $x->valid());