-
-
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
Changes from all commits
72cd437
fc365d4
96e59fd
c7c7395
def3d70
bc965cd
b84701f
1c8c30e
7960a71
d52ebab
36cd094
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
<?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 = null; | ||
|
||
/** | ||
* @param Traversable<TKey,T> $input | ||
*/ | ||
public function __construct(private readonly Traversable $input) {} | ||
|
||
public function getDepth(): int | ||
{ | ||
return null === $this->stack ? -1 : count($this->stack); | ||
} | ||
|
||
/** | ||
* @return Generator<int,T> | ||
*/ | ||
public function getIterator(): Generator | ||
{ | ||
if (null !== $this->stack) { | ||
throw new RuntimeException('Iterator already active '); | ||
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. This cannot happen. This method always returns a new iterator, ready to be consumed. 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.
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. What are you trying to tell me here? 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. Exception will be thrown in this case |
||
} | ||
$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 |
||
$iterator = $this->findIterator($this->input); | ||
|
||
while (true) { | ||
while (null !== $iterator && $iterator->valid() === false) { | ||
$iterator = array_pop($this->stack); | ||
} | ||
|
||
if (null === $iterator) { | ||
$this->stack = null; | ||
|
||
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; | ||
} | ||
} |
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); | ||
} | ||
} |
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