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

Introduce peek_nth #303

Merged
merged 1 commit into from May 7, 2020
Merged

Introduce peek_nth #303

merged 1 commit into from May 7, 2020

Conversation

davidcornu
Copy link
Contributor

I've been implementing Bob Nystrom's Crafting Interpreters in Rust as a way to learn the language and stumbled upon multipeek while looking for a way to peek more than one value ahead (as is required in order to tokenize number literals).

multipeek seemed like a great solution to this but I find it's tricky to use correctly. Because it makes peek stateful, you have to be careful to call reset_peek whenever handing control of the iterator to another part of the code or risk running into bugs when peek doesn't return what you expect.

To work around this I implemented peek_nth which allows you to peek more than one iteration into the future without relying on a cursor. I have a working version in my project but I figured I'd submit it here for discussion.

Example usage

let xs = vec![1,2,3];
let mut iter = peek_nth(xs.iter());

assert_eq!(iter.peek(), Some(&&1));
assert_eq!(iter.next(), Some(&1));

// The iterator does not advance even if we call `peek_nth` multiple times
assert_eq!(iter.peek_nth(0), Some(&&2));
assert_eq!(iter.peek_nth(1), Some(&&3));
assert_eq!(iter.next(), Some(&2));

// Calling `peek_nth` past the end of the iterator will return `None`
assert_eq!(iter.peek_nth(1), None);

Note - peek equivalent to peek_nth(0) and is included for convenience.


⚠️ I'm very new to Rust and took a lot of inspiration frommultipeek to implement this. I'm more than willing to rewrite/fix any mistakes I may have made 😅.

@davidcornu
Copy link
Contributor Author

Ran my code against clippy and fixed all lints.

@bluss
Copy link
Member

bluss commented Nov 18, 2018

Thanks, looks very thoughtful, I'm sorry I haven't had time to look

@davidcornu
Copy link
Contributor Author

No worries! Let me know if I can help in any way.

@jswrenn jswrenn self-assigned this Jul 18, 2019
@davidcornu
Copy link
Contributor Author

@jswrenn is this feature something that would be of interest? Happy to fix the conflicts and get tests passing again if that's the case. Otherwise feel free to close.

@jswrenn
Copy link
Member

jswrenn commented May 3, 2020

@davidcornu This is excellent. I'd be happy to merge it if you have time to fix up the conflicts and get the tests running.

@davidcornu
Copy link
Contributor Author

@jswrenn will do

@davidcornu
Copy link
Contributor Author

Done ✅

src/peek_nth.rs Outdated Show resolved Hide resolved
src/peek_nth.rs Outdated Show resolved Hide resolved
src/peek_nth.rs Outdated Show resolved Hide resolved
src/peek_nth.rs Outdated Show resolved Hide resolved
@phimuemue phimuemue mentioned this pull request May 6, 2020
bors bot added a commit that referenced this pull request May 6, 2020
437: Simpl is empty r=jswrenn a=phimuemue

Taking inspiration from #303 (comment), I think we could exploit that `pop`/`pop_front` return `Option`.

Note: https://godbolt.org/z/WVTD7m seems to indicate that the compiler is doing a good job at optimizing either version.

Co-authored-by: philipp <descpl@yahoo.de>
@davidcornu
Copy link
Contributor Author

@jswrenn thanks for the comments! Definitely cleaned up the implementation.

@jswrenn jswrenn added this to the next milestone May 7, 2020
@jswrenn
Copy link
Member

jswrenn commented May 7, 2020

Thanks for the contribution!

bors r+

@bors
Copy link
Contributor

bors bot commented May 7, 2020

Build succeeded:

@bors bors bot merged commit d081998 into rust-itertools:master May 7, 2020
@davidcornu davidcornu deleted the peek-nth branch October 12, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants