Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Fix problem using IteratorAggregates with skip and take #27

Merged
merged 3 commits into from
Mar 1, 2016

Conversation

robations
Copy link
Contributor

Summary of changes:

  • Added a TestIteratorAggregate class
  • Added test cases for skip and take (plus minor whitespace edit tabs vs spaces)
  • Added IteratorIterator wrapper with explanatory comment

I hope that will do until a safer LimitIterator implementation materialises.

@robations
Copy link
Contributor Author

Fixes #26

@@ -135,7 +136,8 @@ public function take($count)
return new Linq([]);
}

return new Linq(new \LimitIterator($this->iterator, 0, $count));
// IteratorIterator wraps $this->iterator because it might be Traversable but not an Iterator.
Copy link
Member

Choose a reason for hiding this comment

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

Please include this url in the comment "https://bugs.php.net/bug.php?id=52280" so that there is more context.

@@ -134,8 +139,14 @@ public function take($count)
if ($count == 0) {
return new Linq([]);
}
$innerIterator = $this->iterator;
if ($innerIterator instanceof \Iterator === false) {
Copy link
Member

Choose a reason for hiding this comment

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

Please use if (!($innerIterator instanceof \Iterator)) so that we use a consistent coding style (see e.x. https://github.com/fusonic/linq/blob/master/src/Fusonic/Linq/Linq.php#L330)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, no problemo.

@davidroth
Copy link
Member

Looks good to me 👍 I will merge it as soon as there is feedback from Travis.

davidroth added a commit that referenced this pull request Mar 1, 2016
Fix problem using IteratorAggregates with skip and take
@davidroth davidroth merged commit bff72e2 into fusonic:master Mar 1, 2016
@davidroth
Copy link
Member

Thanks!

@robations
Copy link
Contributor Author

No problemo.

@robations robations deleted the hotfix/26 branch March 1, 2016 17:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants