-
Notifications
You must be signed in to change notification settings - Fork 10
Fix problem using IteratorAggregates with skip and take #27
Conversation
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. |
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.
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) { |
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.
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)
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.
Okay, no problemo.
Looks good to me 👍 I will merge it as soon as there is feedback from Travis. |
Fix problem using IteratorAggregates with skip and take
Thanks! |
No problemo. |
Summary of changes:
TestIteratorAggregate
classI hope that will do until a safer LimitIterator implementation materialises.