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
75 changes: 75 additions & 0 deletions src/RecursiveIteratorAggregateIterator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
<?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
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 count($this->stack) - 1;
}

/**
* @return Generator<int,T>
*/
public function getIterator(): Generator
{
$this->stack = [self::findIterator($this->input)];

while (true) {
while (count($this->stack) > 0 && !end($this->stack)->valid()) {
array_pop($this->stack);
}

if (count($this->stack) === 0) {
mastir marked this conversation as resolved.
Show resolved Hide resolved
return;
}
$current = end($this->stack)->current();

yield $current;
end($this->stack)->next();
Copy link
Contributor

Choose a reason for hiding this comment

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

end() is a $\mathcal{O}(n)$ operation, it may be slow, let's try to call it once here.

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 simple have last $iterator outside of a $stack to avoid array operations

$this->stack[] = self::findIterator($current);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not need to have a static function self::findIterator, let's remove the static from it.

}
}

private static 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);
}
}
Loading