Skip to content
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
35 changes: 22 additions & 13 deletions phpstan-baseline.neon
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
85 changes: 85 additions & 0 deletions 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
Copy link
Contributor

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 the IteratorAggregate interface, I would remove it.

Copy link
Author

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

Copy link
Contributor

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.

Copy link
Author

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

foreach($it as $val){
   echo str_repeat('-', $it->getDepth()).$val."\n";
}

Copy link
Contributor

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.

Copy link
Author

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

$prev_depth = 0;
foreach($it as $val){
   $next_depth = $it->getDepth();
   if ($prev_depth < $next_depth) echo "<tag>\n";
   for($i=$prev_depth;$i>$next_depth;$i--) echo "</tag>\n";
   echo $val;
   $prev_depth = $next_depth;
}
for($i=$prev_depth;$i>0;$i--) echo "</tag>\n";

Copy link
Author

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

{
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 ');
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

foreach($it as $a){ //generator assign stack to iterator
    foreach($it as $b){ //same iterator, new stack will replace original, throw exception
    }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you trying to tell me here?

Copy link
Author

@mastir mastir Jul 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception will be thrown in this case

}
$this->stack = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's keep the object immutable, this should not happen

Copy link
Author

Choose a reason for hiding this comment

The 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;
}
}
61 changes: 61 additions & 0 deletions tests/unit/RecursiveIteratorAggregateIteratorTest.php
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);
}
}