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 peekable_map_while and peekable_take_while functions and associated structs #92665

Closed
wants to merge 2 commits into from

Conversation

devins2518
Copy link

These iterators allow you to use the map_while and take_while functions without needing to consume self. While implementing these, I discovered that both MapWhile and TakeWhile have custom implementations of fold and try_fold which I did not implement for PeekableMapWhile and PeekableTakeWhile as I didn't understand their purpose.

Any insights would be appreciated. Thank you.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 8, 2022
@the8472
Copy link
Member

the8472 commented Jan 8, 2022

The default implementations for fold and try_fold are usually overridden for performance reasons

The names could use some improvement since it repeats itself peekable() and then peekable_take_while() and it's also quite long. Maybe peek_while? No idea for peekable_map_while though.

I'm wondering whether we need this API at all in the long run. If specialization were closer to stabilization then we could simply do peekable().by_ref().map_while() and specialize that implementation to keep the peeked value when the predicate returns false, then one can drop the MapWhile and get back access to the Peekable. Although that would be a breaking change in behavior, so probably not.

@devins2518
Copy link
Author

I've just realized that Itertools provides a similar iterator as PeekingTakeWhile. Should this PR be closed in favor of using that until specialization lands?

@the8472
Copy link
Member

the8472 commented Jan 8, 2022

The specialization idea wasn't that great so I don't think we should really wait for that. But if itertools already has it then that may be good enough, it does seem kind of niche.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants