-
-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add RecursiveIteratorAggregateIterator #56
Closed
mastir
wants to merge
11
commits into
loophp:main
from
mastir:mastir-RecursiveIteratorAggregateIterator
Closed
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
72cd437
add RecursiveIteratorAggregateIterator
mastir fc365d4
add test, run cs
mastir 96e59fd
fix static analyse
mastir c7c7395
move phpstan-ignore to baseline
mastir def3d70
updated phpstan-baseline
mastir bc965cd
RecursiveIteratorAggregateIterator move stack initialisation from con…
mastir b84701f
Update src/RecursiveIteratorAggregateIterator.php
mastir 1c8c30e
optimise RecursiveIteratorAggregateIterator
mastir 7960a71
RecursiveIteratorAggregateIterator make findIterator not static
mastir d52ebab
Merge
mastir 36cd094
add exception on secondary iteration start
mastir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,25 @@ | ||
parameters: | ||
ignoreErrors: | ||
- | ||
message: "#^While loop condition is always true\\.$#" | ||
count: 1 | ||
path: src/RandomIntegerAggregate.php | ||
ignoreErrors: | ||
- | ||
message: "#^While loop condition is always true\\.$#" | ||
count: 1 | ||
path: src/RandomIntegerAggregate.php | ||
|
||
- | ||
message: "#^Parameter \\#1 \\$iterable of class loophp\\\\iterators\\\\UnpackIterableAggregate constructor expects iterable\\<\\(int\\|string\\), array\\{TKey, T\\}\\>, loophp\\\\iterators\\\\UnpackIterableAggregate\\<array\\{TKey, T\\}\\|int\\|null, array\\{TKey, T\\}\\|int\\|null\\> given\\.$#" | ||
count: 1 | ||
path: src/RandomIterableAggregate.php | ||
- | ||
message: "#^Parameter \\#1 \\$iterable of class loophp\\\\iterators\\\\UnpackIterableAggregate constructor expects iterable\\<\\(int\\|string\\), array\\{TKey, T\\}\\>, loophp\\\\iterators\\\\UnpackIterableAggregate\\<array\\{TKey, T\\}\\|int\\|null, array\\{TKey, T\\}\\|int\\|null\\> given\\.$#" | ||
count: 1 | ||
path: src/RandomIterableAggregate.php | ||
|
||
- | ||
message: "#^Parameter \\#1 \\$iterable of class loophp\\\\iterators\\\\UnpackIterableAggregate constructor expects iterable\\<\\(int\\|string\\), array\\{array\\{TKey, T\\}\\|int\\|null, array\\{TKey, T\\}\\|int\\|null\\}\\>, loophp\\\\iterators\\\\SortIterableAggregate\\<array\\<int, int\\|null\\>, array\\<int, array\\{TKey, T\\}\\|int\\|null\\>\\> given\\.$#" | ||
count: 1 | ||
path: src/RandomIterableAggregate.php | ||
- | ||
message: "#^Parameter \\#1 \\$iterable of class loophp\\\\iterators\\\\UnpackIterableAggregate constructor expects iterable\\<\\(int\\|string\\), array\\{array\\{TKey, T\\}\\|int\\|null, array\\{TKey, T\\}\\|int\\|null\\}\\>, loophp\\\\iterators\\\\SortIterableAggregate\\<array\\<int, int\\|null\\>, array\\<int, array\\{TKey, T\\}\\|int\\|null\\>\\> given\\.$#" | ||
count: 1 | ||
path: src/RandomIterableAggregate.php | ||
- | ||
message: "#^Method loophp\\\\iterators\\\\RecursiveIteratorAggregateIterator\\:\\:findIterator\\(\\) has parameter \\$input with no value type specified in iterable type Traversable\\.$#" | ||
count: 1 | ||
path: src/RecursiveIteratorAggregateIterator.php | ||
|
||
- | ||
message: "#^Method loophp\\\\iterators\\\\RecursiveIteratorAggregateIterator\\:\\:findIterator\\(\\) should return Iterator but returns Traversable\\.$#" | ||
count: 1 | ||
path: src/RecursiveIteratorAggregateIterator.php |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace loophp\iterators; | ||
|
||
use Generator; | ||
use Iterator; | ||
use IteratorAggregate; | ||
use RuntimeException; | ||
use Traversable; | ||
|
||
use function count; | ||
|
||
/** | ||
* @template TKey | ||
* @template T | ||
* | ||
* @implements IteratorAggregate<int, T> | ||
*/ | ||
class RecursiveIteratorAggregateIterator implements IteratorAggregate | ||
{ | ||
/** | ||
* @var list<Iterator> | ||
*/ | ||
private array $stack = []; | ||
|
||
/** | ||
* @param Traversable<TKey,T> $input | ||
*/ | ||
public function __construct(private readonly Traversable $input) {} | ||
|
||
public function getDepth(): int | ||
{ | ||
return count($this->stack); | ||
} | ||
|
||
/** | ||
* @return Generator<int,T> | ||
*/ | ||
public function getIterator(): Generator | ||
{ | ||
$iterator = $this->findIterator($this->input); | ||
$this->stack = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep the object immutable, this should not happen There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can`t have state tracking (getDepth) on immutable iterator, we can add exception here |
||
|
||
while (true) { | ||
while (null !== $iterator && $iterator->valid() === false) { | ||
$iterator = array_pop($this->stack); | ||
} | ||
|
||
if (null === $iterator) { | ||
return; | ||
} | ||
$current = $iterator->current(); | ||
|
||
yield $current; | ||
$iterator->next(); | ||
|
||
if ($current instanceof Traversable) { | ||
$this->stack[] = $iterator; | ||
$iterator = $this->findIterator($current); | ||
} | ||
} | ||
} | ||
|
||
private function findIterator(Traversable $input): Iterator | ||
{ | ||
$prev = null; | ||
|
||
while (!($input instanceof Iterator)) { | ||
if ($prev === $input || !($input instanceof IteratorAggregate)) { | ||
throw new RuntimeException('Invalid iterator'); | ||
} | ||
$prev = $input; | ||
$input = $input->getIterator(); | ||
} | ||
|
||
return $input; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace tests\loophp\iterators; | ||
|
||
use ArrayIterator; | ||
use IteratorAggregate; | ||
use loophp\iterators\RecursiveIteratorAggregateIterator; | ||
use PHPUnit\Framework\TestCase; | ||
use Traversable; | ||
|
||
class RecursiveIteratorAggregateIteratorTestObject implements IteratorAggregate | ||
{ | ||
public function __construct(public readonly string $name, public readonly array $items) {} | ||
|
||
public function getIterator(): Traversable | ||
{ | ||
return new ArrayIterator($this->items); | ||
} | ||
} | ||
|
||
/** | ||
* @internal | ||
* | ||
* @coversNothing | ||
*/ | ||
final class RecursiveIteratorAggregateIteratorTest extends TestCase | ||
{ | ||
public function testRecursiveIteratorAggregateIterator() | ||
{ | ||
$input = [ | ||
new RecursiveIteratorAggregateIteratorTestObject('1', [ | ||
new RecursiveIteratorAggregateIteratorTestObject('1-1', [ | ||
new RecursiveIteratorAggregateIteratorTestObject('1-1-1', []), | ||
]), | ||
new RecursiveIteratorAggregateIteratorTestObject('1-2', []), | ||
]), | ||
new RecursiveIteratorAggregateIteratorTestObject('2', []), | ||
new RecursiveIteratorAggregateIteratorTestObject('3', [ | ||
new RecursiveIteratorAggregateIteratorTestObject('3-1', []), | ||
]), | ||
]; | ||
$expected = [ | ||
'0: 1', | ||
'1: 1-1', | ||
'2: 1-1-1', | ||
'1: 1-2', | ||
'0: 2', | ||
'0: 3', | ||
'1: 3-1', | ||
]; | ||
$result = []; | ||
$iterator = new RecursiveIteratorAggregateIterator(new ArrayIterator($input)); | ||
|
||
foreach ($iterator as $item) { | ||
$result[] = $iterator->getDepth() . ': ' . $item->name; | ||
} | ||
self::assertSame($expected, $result); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
getDepth
method is not part of theIteratorAggregate
interface, I would remove it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getDepth method is required for tree travel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then it should be private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It`s not used by iterator, its required to detect depth (or move direction) in loop. Like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I noticed it after your message. Since that method is not part of the interface, I would make it private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or more common use case for xml/html/whatever
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private method cannot be called outside of the iterator class (including foreach loop). It`s simple useless