Skip to content

Add a "peekable" iterator adaptor, with a peek() method that returns the next element. #8428

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

Closed
wants to merge 3 commits into from
Closed

Conversation

bluss
Copy link
Member

@bluss bluss commented Aug 10, 2013

Peekable changes by @SimonSapin and original PR is #8396

@bluss
Copy link
Member Author

bluss commented Aug 10, 2013

The naming collision is now real, with IteratorUtil and Iterator merged, the old .peek() and new .peek() have the same method name.

  • Remove the old peek since it's pretty useless (can be accomplished with1 filter)?
  • Rename one of the two .peek? I was thinking of the slightly cheeky .manhole() for the preexisting .peek()

blake2-ppc and others added 3 commits August 11, 2013 01:57
Replace the previous equivalent iterator adaptor with .peekable().
Refactor the set operation iterators so that they are easier to read.
@bluss
Copy link
Member Author

bluss commented Aug 11, 2013

Rebased to cope with the (awesome) iterator changes merged inbetween. I squashed the rename of PeekableIterator -> Peekable together with the introduction of peekable, to make conflict resolution when rebasing easier.

@Kimundi
Copy link
Member

Kimundi commented Aug 11, 2013

Hm, just an idea: This Iterator basically buffers one element for the peek() function, right?

We could just have a generic BufferIterator that buffers N elements, and that has the ability to peek N elements into the future. Of course, that would either have to allocate for the internal vector, or require uints in the typesystem so that it can use embedded [T, ..N] vectors, so it's probably not worth doing right now.

@bluss
Copy link
Member Author

bluss commented Aug 11, 2013

I think lookahead or peek is the better name, but sure it's an idea kimundi, but we can't implement it easily.

@SimonSapin
Copy link
Contributor

@Kimundi it only buffers when you call peek(). If you don’t, self.peeked is always None and next() just defers to self.iter.next(). I suppose we could make the code simpler by always buffering one element, but that’s consuming the underlying iterator more than we have to. I don’t know which is better.

As to peek(n), yes that could be useful as well but as you say not as obvious to implement. (I also personally don’t need it yet.)

bors added a commit that referenced this pull request Aug 12, 2013
@bors bors closed this Aug 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants