From 8e003c0c0bf3943a138eee80af5dbae19f4930a2 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 6 Jul 2023 10:12:43 +0200 Subject: [PATCH 1/6] Pass key to callback in CallbackIterator --- src/Model/CallbackIterator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/CallbackIterator.php b/src/Model/CallbackIterator.php index a900cd453..e8f7d6b82 100644 --- a/src/Model/CallbackIterator.php +++ b/src/Model/CallbackIterator.php @@ -49,7 +49,7 @@ public function __construct(Traversable $traversable, Closure $callback) #[ReturnTypeWillChange] public function current() { - return ($this->callback)($this->iterator->current()); + return ($this->callback)($this->iterator->current(), $this->iterator->key()); } /** From 66ca4a414b03d28f409810e505300c364baaab73 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 6 Jul 2023 11:15:40 +0200 Subject: [PATCH 2/6] Add template params to CallbackIterator --- src/Model/CallbackIterator.php | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Model/CallbackIterator.php b/src/Model/CallbackIterator.php index e8f7d6b82..ba2df4b52 100644 --- a/src/Model/CallbackIterator.php +++ b/src/Model/CallbackIterator.php @@ -27,15 +27,24 @@ * Iterator to apply a callback before returning an element * * @internal + * + * @template TKey + * @template TValue + * @template TCallbackValue + * @template-implements Iterator */ class CallbackIterator implements Iterator { - /** @var Closure */ + /** @var Closure(TValue, TKey): TCallbackValue */ private $callback; - /** @var Iterator */ + /** @var Iterator */ private $iterator; + /** + * @param Traversable $traversable + * @param Closure(TValue, TKey): TCallbackValue $callback + */ public function __construct(Traversable $traversable, Closure $callback) { $this->iterator = $traversable instanceof Iterator ? $traversable : new IteratorIterator($traversable); @@ -44,7 +53,7 @@ public function __construct(Traversable $traversable, Closure $callback) /** * @see https://php.net/iterator.current - * @return mixed + * @return TCallbackValue */ #[ReturnTypeWillChange] public function current() @@ -54,7 +63,7 @@ public function current() /** * @see https://php.net/iterator.key - * @return mixed + * @return TKey */ #[ReturnTypeWillChange] public function key() From 3bac4027421451f48f31830485f2ab6cbb5ec55e Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 7 Jul 2023 13:46:01 +0200 Subject: [PATCH 3/6] Add tests for CallbackIterator --- tests/Model/CallbackIteratorTest.php | 52 ++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 tests/Model/CallbackIteratorTest.php diff --git a/tests/Model/CallbackIteratorTest.php b/tests/Model/CallbackIteratorTest.php new file mode 100644 index 000000000..921df10de --- /dev/null +++ b/tests/Model/CallbackIteratorTest.php @@ -0,0 +1,52 @@ +assertSame($expectedKey, $key); + $expectedKey++; + + return $value * 2; + } + ); + + $this->assertSame([2, 4, 6], iterator_to_array($callbackIterator)); + } + + public function testHashIteration(): void + { + $expectedKey = 0; + + $original = ['a' => 1, 'b' => 2, 'c' => 3]; + $expectedKeys = array_keys($original); + + $callbackIterator = new CallbackIterator( + new ArrayIterator($original), + function ($value, $key) use (&$expectedKey, $expectedKeys) { + $this->assertSame($expectedKeys[$expectedKey], $key); + $expectedKey++; + + return $value * 2; + } + ); + + $this->assertSame(['a' => 2, 'b' => 4, 'c' => 6], iterator_to_array($callbackIterator)); + } +} From 56a33c1d46ea9cea864dc218684dda5974f01c68 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 7 Jul 2023 13:51:42 +0200 Subject: [PATCH 4/6] Accept any callable as callback in CallbackIterator --- src/Model/CallbackIterator.php | 17 +++++++++-------- tests/Model/CallbackIteratorTest.php | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/Model/CallbackIterator.php b/src/Model/CallbackIterator.php index ba2df4b52..5c01f936c 100644 --- a/src/Model/CallbackIterator.php +++ b/src/Model/CallbackIterator.php @@ -17,12 +17,13 @@ namespace MongoDB\Model; -use Closure; use Iterator; use IteratorIterator; use ReturnTypeWillChange; use Traversable; +use function call_user_func; + /** * Iterator to apply a callback before returning an element * @@ -35,20 +36,20 @@ */ class CallbackIterator implements Iterator { - /** @var Closure(TValue, TKey): TCallbackValue */ - private $callback; + /** @var callable(TValue, TKey): TCallbackValue */ + private $callable; /** @var Iterator */ private $iterator; /** - * @param Traversable $traversable - * @param Closure(TValue, TKey): TCallbackValue $callback + * @param Traversable $traversable + * @param callable(TValue, TKey): TCallbackValue $callable */ - public function __construct(Traversable $traversable, Closure $callback) + public function __construct(Traversable $traversable, callable $callable) { $this->iterator = $traversable instanceof Iterator ? $traversable : new IteratorIterator($traversable); - $this->callback = $callback; + $this->callable = $callable; } /** @@ -58,7 +59,7 @@ public function __construct(Traversable $traversable, Closure $callback) #[ReturnTypeWillChange] public function current() { - return ($this->callback)($this->iterator->current(), $this->iterator->key()); + return call_user_func($this->callable, $this->iterator->current(), $this->iterator->key()); } /** diff --git a/tests/Model/CallbackIteratorTest.php b/tests/Model/CallbackIteratorTest.php index 921df10de..a070fa871 100644 --- a/tests/Model/CallbackIteratorTest.php +++ b/tests/Model/CallbackIteratorTest.php @@ -8,6 +8,7 @@ use function array_keys; use function iterator_to_array; +use function strrev; class CallbackIteratorTest extends TestCase { @@ -49,4 +50,21 @@ function ($value, $key) use (&$expectedKey, $expectedKeys) { $this->assertSame(['a' => 2, 'b' => 4, 'c' => 6], iterator_to_array($callbackIterator)); } + + public function testWithCallable(): void + { + $original = ['foo', 'bar', 'baz']; + + $callbackIterator = new CallbackIterator( + new ArrayIterator($original), + [self::class, 'reverseValue'] + ); + + $this->assertSame(['oof', 'rab', 'zab'], iterator_to_array($callbackIterator)); + } + + public static function reverseValue($value, $key) + { + return strrev($value); + } } From f3d1ddc46fc06a960db1dd0cfd80dfd311452185 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 10 Jul 2023 09:50:15 +0200 Subject: [PATCH 5/6] Rename callback variable to convey purpose --- src/Model/CallbackIterator.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Model/CallbackIterator.php b/src/Model/CallbackIterator.php index 5c01f936c..49d04ce48 100644 --- a/src/Model/CallbackIterator.php +++ b/src/Model/CallbackIterator.php @@ -37,19 +37,19 @@ class CallbackIterator implements Iterator { /** @var callable(TValue, TKey): TCallbackValue */ - private $callable; + private $callback; /** @var Iterator */ private $iterator; /** * @param Traversable $traversable - * @param callable(TValue, TKey): TCallbackValue $callable + * @param callable(TValue, TKey): TCallbackValue $callback */ - public function __construct(Traversable $traversable, callable $callable) + public function __construct(Traversable $traversable, callable $callback) { $this->iterator = $traversable instanceof Iterator ? $traversable : new IteratorIterator($traversable); - $this->callable = $callable; + $this->callback = $callback; } /** @@ -59,7 +59,7 @@ public function __construct(Traversable $traversable, callable $callable) #[ReturnTypeWillChange] public function current() { - return call_user_func($this->callable, $this->iterator->current(), $this->iterator->key()); + return call_user_func($this->callback, $this->iterator->current(), $this->iterator->key()); } /** From 481812496867e4ef149f1e1627bfd507dbcc8ec4 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 10 Jul 2023 09:50:22 +0200 Subject: [PATCH 6/6] Rework CallbackIterator test to use data providers --- tests/Model/CallbackIteratorTest.php | 95 ++++++++++++++++------------ 1 file changed, 55 insertions(+), 40 deletions(-) diff --git a/tests/Model/CallbackIteratorTest.php b/tests/Model/CallbackIteratorTest.php index a070fa871..efa3bd7d7 100644 --- a/tests/Model/CallbackIteratorTest.php +++ b/tests/Model/CallbackIteratorTest.php @@ -3,68 +3,83 @@ namespace MongoDB\Tests\Model; use ArrayIterator; +use Generator; +use Iterator; +use IteratorAggregate; use MongoDB\Model\CallbackIterator; use MongoDB\Tests\TestCase; -use function array_keys; use function iterator_to_array; -use function strrev; class CallbackIteratorTest extends TestCase { - public function testArrayIteration(): void + /** @dataProvider provideTests */ + public function testIteration($expected, $source, $callback): void { - $expectedKey = 0; + $callbackIterator = new CallbackIterator($source, $callback); - $original = [1, 2, 3]; - - $callbackIterator = new CallbackIterator( - new ArrayIterator($original), - function ($value, $key) use (&$expectedKey) { - $this->assertSame($expectedKey, $key); - $expectedKey++; - - return $value * 2; - } - ); - - $this->assertSame([2, 4, 6], iterator_to_array($callbackIterator)); + $this->assertEquals($expected, iterator_to_array($callbackIterator)); } - public function testHashIteration(): void + public static function provideTests(): Generator { - $expectedKey = 0; + $listIterator = new ArrayIterator([1, 2, 3]); + $hashIterator = new ArrayIterator(['a' => 1, 'b' => 2, 'c' => 3]); - $original = ['a' => 1, 'b' => 2, 'c' => 3]; - $expectedKeys = array_keys($original); + $iteratorAggregate = new class ($listIterator) implements IteratorAggregate + { + private $iterator; - $callbackIterator = new CallbackIterator( - new ArrayIterator($original), - function ($value, $key) use (&$expectedKey, $expectedKeys) { - $this->assertSame($expectedKeys[$expectedKey], $key); - $expectedKey++; + public function __construct(Iterator $iterator) + { + $this->iterator = $iterator; + } - return $value * 2; + public function getIterator(): Iterator + { + return $this->iterator; } - ); + }; - $this->assertSame(['a' => 2, 'b' => 4, 'c' => 6], iterator_to_array($callbackIterator)); - } + yield 'List with closure' => [ + 'expected' => [2, 4, 6], + 'source' => $listIterator, + 'callback' => function ($value, $key) use ($listIterator) { + self::assertSame($listIterator->key(), $key); - public function testWithCallable(): void - { - $original = ['foo', 'bar', 'baz']; + return $value * 2; + }, + ]; - $callbackIterator = new CallbackIterator( - new ArrayIterator($original), - [self::class, 'reverseValue'] - ); + yield 'List with callable' => [ + 'expected' => [2, 4, 6], + 'source' => $listIterator, + 'callback' => [self::class, 'doubleValue'], + ]; - $this->assertSame(['oof', 'rab', 'zab'], iterator_to_array($callbackIterator)); + yield 'Hash with closure' => [ + 'expected' => ['a' => 2, 'b' => 4, 'c' => 6], + 'source' => $hashIterator, + 'callback' => function ($value, $key) use ($hashIterator) { + self::assertSame($hashIterator->key(), $key); + + return $value * 2; + }, + ]; + + yield 'IteratorAggregate with closure' => [ + 'expected' => [2, 4, 6], + 'source' => $iteratorAggregate, + 'callback' => function ($value, $key) use ($listIterator) { + self::assertSame($listIterator->key(), $key); + + return $value * 2; + }, + ]; } - public static function reverseValue($value, $key) + public static function doubleValue($value, $key) { - return strrev($value); + return $value * 2; } }