From f648f7752189d718516902610a91139f1634d603 Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Fri, 24 Jun 2022 16:05:32 +0200 Subject: [PATCH 1/6] Improving performance by implementing IteratorAggregate --- composer.json | 5 +++ src/Iterator/ColumnIterator.php | 72 ++++++++++++--------------------- src/Iterator/FlipIterator.php | 23 +++++++---- src/Iterator/KeysIterator.php | 26 +++++++++--- src/Iterator/MapIterator.php | 34 +++++++--------- src/Iterator/ValuesIterator.php | 40 ++++++------------ 6 files changed, 95 insertions(+), 105 deletions(-) diff --git a/composer.json b/composer.json index 88be391..240f89d 100644 --- a/composer.json +++ b/composer.json @@ -17,6 +17,11 @@ "src/iterator_functions.php" ] }, + "autoload-dev": { + "psr-4": { + "DoekeNorg\\IteratorFunctions\\Tests\\": "tests" + } + }, "license": "MIT", "authors": [ { diff --git a/src/Iterator/ColumnIterator.php b/src/Iterator/ColumnIterator.php index ba2f8ef..1a05c82 100644 --- a/src/Iterator/ColumnIterator.php +++ b/src/Iterator/ColumnIterator.php @@ -5,7 +5,7 @@ /** * Iterator that returns a single column for the iteration array / object. */ -class ColumnIterator extends \FilterIterator +class ColumnIterator implements \IteratorAggregate { /** * The column to return. @@ -25,6 +25,12 @@ class ColumnIterator extends \FilterIterator */ private int $count = 0; + /** + * The inner iterator. + * @var \Traversable + */ + private \Traversable $iterator; + /** * Creates the iterator. * @param \Traversable $iterator The iterator that provides the arrays / objects. @@ -33,56 +39,19 @@ class ColumnIterator extends \FilterIterator */ public function __construct(\Traversable $iterator, $column_key, $index_key = null) { - parent::__construct($iterator); - + $this->iterator = $iterator; $this->column_key = $column_key; $this->index_key = $index_key; } - /** - * @inheritdoc - */ - public function next(): void - { - parent::next(); - - if (isset($this->index_key) && !$this->column($this->index_key)) { - ++$this->count; - } - } - - /** - * @inheritdoc - */ - public function key() - { - if (isset($this->index_key)) { - return $this->column($this->index_key) ?? $this->count; - } - - return parent::key(); - } - - /** - * @inheritdoc - */ - public function current() - { - if (isset($this->column_key)) { - return $this->column($this->column_key); - } - - return parent::current(); - } - /** * Retrieves a single column from the current iteration. + * @param array|\ArrayAccess|object $iteration The iteration. * @param int|string $key The key to retrieve from the iteration. * @return mixed|null The value. */ - protected function column($key) + private function getColumn($iteration, $key) { - $iteration = parent::current(); if (is_array($iteration) || $iteration instanceof \ArrayAccess) { return $iteration[$key] ?? null; } @@ -95,14 +64,23 @@ protected function column($key) } /** - * @inheritdoc + * @inheritDoc */ - public function accept(): bool + public function getIterator(): \Generator { - if (isset($this->column_key)) { - return $this->column($this->column_key) !== null; + foreach ($this->iterator as $key => $iteration) { + if (isset($this->index_key)) { + $key = $this->getColumn($iteration, $this->index_key) ?? $this->count++; + } + + if (isset($this->column_key)) { + $iteration = $this->getColumn($iteration, $this->column_key); + if ($iteration === null) { + continue; + } + } + + yield $key => $iteration; } - - return true; } } diff --git a/src/Iterator/FlipIterator.php b/src/Iterator/FlipIterator.php index e3ffb5f..f701ebc 100644 --- a/src/Iterator/FlipIterator.php +++ b/src/Iterator/FlipIterator.php @@ -5,21 +5,30 @@ /** * Iterator that flips the key and the value of the current iteration. */ -class FlipIterator extends \IteratorIterator +class FlipIterator implements \IteratorAggregate { /** - * @inheritdoc + * The inner iterator. + * @var \Traversable */ - public function key() + private \Traversable $iterator; + + /** + * Creates the iterator. + * @param \Traversable $iterator The inner iterator. + */ + public function __construct(\Traversable $iterator) { - return parent::current(); + $this->iterator = $iterator; } /** - * @inheritdoc + * @inheritDoc */ - public function current() + public function getIterator(): \Generator { - return parent::key(); + foreach ($this->iterator as $key => $value) { + yield $value => $key; + } } } diff --git a/src/Iterator/KeysIterator.php b/src/Iterator/KeysIterator.php index cbbab3e..009b26e 100644 --- a/src/Iterator/KeysIterator.php +++ b/src/Iterator/KeysIterator.php @@ -5,14 +5,30 @@ /** * Iterator that returns only the keys of the provided iterator. */ -class KeysIterator extends ValuesIterator +class KeysIterator implements \IteratorAggregate { /** - * @inheritdoc - * @psalm-suppress UndefinedInterfaceMethod Psalm doesn't understand the inner iterator is an {@see \Iterator}. + * The inner iterator. + * @var \Traversable */ - public function current() + private \Traversable $iterator; + + /** + * Creates the iterator. + * @param \Traversable $iterator The inner iterator. + */ + public function __construct(\Traversable $iterator) + { + $this->iterator = $iterator; + } + + /** + * @inheritDoc + */ + public function getIterator(): \Generator { - return $this->getInnerIterator()->key(); + foreach ($this->iterator as $key => $value) { + yield $key; + } } } diff --git a/src/Iterator/MapIterator.php b/src/Iterator/MapIterator.php index 4815767..4194e8a 100644 --- a/src/Iterator/MapIterator.php +++ b/src/Iterator/MapIterator.php @@ -5,7 +5,7 @@ /** * Iterator that applies a callback to the elements of the given iterators. */ -class MapIterator extends \IteratorIterator +class MapIterator implements \IteratorAggregate { /** * Callback function to run for each element in each iterator. @@ -13,6 +13,12 @@ class MapIterator extends \IteratorIterator */ private $callback; + /** + * The inner iterator. + * @var \MultipleIterator + */ + private \MultipleIterator $iterator; + /** * Returns the iterator. * @param array|\Iterator ...$iterators Any iterator to apply the callback on. Can also be an array. @@ -23,41 +29,31 @@ public function __construct(callable $callback, iterable ...$iterators) try { $function = new \ReflectionFunction(\Closure::fromCallable($callback)); } catch (\ReflectionException $e) { - throw new \RuntimeException($e->getMessage(), (int) $e->getCode(), $e); + throw new \RuntimeException($e->getMessage(), (int)$e->getCode(), $e); } if (!$function->isVariadic() && $function->getNumberOfParameters() !== count($iterators)) { throw new \InvalidArgumentException('The callback needs as many arguments as provided iterators.'); } - $inner = new \MultipleIterator(); + $this->iterator = new \MultipleIterator(); foreach ($iterators as $iterator) { if (is_array($iterator)) { $iterator = new \ArrayIterator($iterator); } - $inner->attachIterator($iterator); + $this->iterator->attachIterator($iterator); } - parent::__construct($inner); - $this->callback = $callback; } /** - * @inheritdoc + * @inheritDoc */ - public function key() + public function getIterator(): \Generator { - $keys = parent::key(); - - return is_array($keys) ? reset($keys) : parent::key(); - } - - /** - * @inheritdoc - */ - public function current() - { - return call_user_func_array($this->callback, parent::current()); + foreach ($this->iterator as $key => $value) { + yield $key[0] => ($this->callback)(...$value); + } } } diff --git a/src/Iterator/ValuesIterator.php b/src/Iterator/ValuesIterator.php index e6b9740..265ddd7 100644 --- a/src/Iterator/ValuesIterator.php +++ b/src/Iterator/ValuesIterator.php @@ -5,44 +5,30 @@ /** * Iterator that returns only the values of the provided iterator. */ -class ValuesIterator extends \IteratorIterator +class ValuesIterator implements \IteratorAggregate { /** - * The internal key count. - * @var int + * The inner iterator. + * @var \Traversable */ - private int $count = 0; + private \Traversable $iterator; /** - * @inheritdoc + * Creates the iterator. + * @param \Traversable $iterator The inner iterator. */ - public function key(): int + public function __construct(\Traversable $iterator) { - return $this->count; + $this->iterator = $iterator; } /** - * @inheritdoc - * - * Increases the internal count. - * + * @inheritDoc */ - public function next(): void + public function getIterator(): \Generator { - parent::next(); - ++$this->count; - } - - /** - * @inheritdoc - * - * Resets the internal count. - * - */ - public function rewind(): void - { - parent::rewind(); - - $this->count = 0; + foreach ($this->iterator as $value) { + yield $value; + } } } From 157df115eb8528aab354b7802553573033535525 Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Fri, 24 Jun 2022 16:19:23 +0200 Subject: [PATCH 2/6] psalm fixes --- src/Iterator/ColumnIterator.php | 4 ++-- src/Iterator/KeysIterator.php | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Iterator/ColumnIterator.php b/src/Iterator/ColumnIterator.php index 1a05c82..1443c0a 100644 --- a/src/Iterator/ColumnIterator.php +++ b/src/Iterator/ColumnIterator.php @@ -48,7 +48,7 @@ public function __construct(\Traversable $iterator, $column_key, $index_key = nu * Retrieves a single column from the current iteration. * @param array|\ArrayAccess|object $iteration The iteration. * @param int|string $key The key to retrieve from the iteration. - * @return mixed|null The value. + * @return mixed The value. */ private function getColumn($iteration, $key) { @@ -57,7 +57,7 @@ private function getColumn($iteration, $key) } if (is_object($iteration)) { - return $iteration->{$key} ?? null; + return $iteration->{$key} ?: null; } return null; diff --git a/src/Iterator/KeysIterator.php b/src/Iterator/KeysIterator.php index 009b26e..ae41c60 100644 --- a/src/Iterator/KeysIterator.php +++ b/src/Iterator/KeysIterator.php @@ -27,7 +27,7 @@ public function __construct(\Traversable $iterator) */ public function getIterator(): \Generator { - foreach ($this->iterator as $key => $value) { + foreach ($this->iterator as $key => $_) { yield $key; } } From 1842232eafa834274fd9074ac7513ff360c2280d Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Fri, 24 Jun 2022 16:21:20 +0200 Subject: [PATCH 3/6] changelog update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7469843..a91d77a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed - Added `.gitattributes` to reduce disk usage. +- Reimplemented using `IteratorAggregate` to improve performance. ## [1.0.1] - 2021-08-29 ### Changed From dd2e4a33b2c579bafa978b5c014505fee3bff012 Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Fri, 24 Jun 2022 16:27:16 +0200 Subject: [PATCH 4/6] improved testsuite --- .github/workflows/run-tests.yml | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 3ecb74d..7acd33e 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -1,6 +1,6 @@ name: Tests -on: [push, pull_request] +on: [ pull_request ] jobs: test: @@ -8,11 +8,10 @@ jobs: strategy: fail-fast: true matrix: - os: [ubuntu-latest, windows-latest] - php: [7.4, 8.0] - dependency-version: [prefer-lowest, prefer-stable] + os: [ ubuntu-latest, windows-latest ] + php: [ 7.4, 8.0 ] - name: P${{ matrix.php }} - ${{ matrix.dependency-version }} - ${{ matrix.os }} + name: PHP ${{ matrix.php }} - ${{ matrix.os }} steps: - name: Checkout code @@ -24,8 +23,14 @@ jobs: php-version: ${{ matrix.php }} coverage: none + - name: Cache composer dependencies + uses: actions/cache@v2 + with: + path: vendor + key: composer-${{ matrix.php }}-${{ matrix.os }}-${{ hashFiles('composer.lock') }} + - name: Install dependencies - run: composer update --${{ matrix.dependency-version }} --prefer-dist --no-interaction + run: composer update --prefer-dist --no-interaction - name: Execute tests - run: vendor/bin/pest \ No newline at end of file + run: vendor/bin/pest From 72d9b5359ed47949294556dd52deea08e317623e Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Fri, 24 Jun 2022 16:30:08 +0200 Subject: [PATCH 5/6] Cache-test From 66f28061709f6b5a509a5b361009591c9c63e8a2 Mon Sep 17 00:00:00 2001 From: Doeke Norg Date: Fri, 24 Jun 2022 16:34:46 +0200 Subject: [PATCH 6/6] only run install on invalid cache --- .github/workflows/run-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index 7acd33e..d6a6715 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -24,12 +24,14 @@ jobs: coverage: none - name: Cache composer dependencies + id: cache-run uses: actions/cache@v2 with: path: vendor key: composer-${{ matrix.php }}-${{ matrix.os }}-${{ hashFiles('composer.lock') }} - name: Install dependencies + if: steps.cache-run.outputs.cache-hit != 'true' run: composer update --prefer-dist --no-interaction - name: Execute tests