Skip to content

Comments

refactor: partially refactor iter types into submodules#1531

Draft
demosdemon wants to merge 1 commit intomainfrom
brandon.leblanc/refactor-iter-types
Draft

refactor: partially refactor iter types into submodules#1531
demosdemon wants to merge 1 commit intomainfrom
brandon.leblanc/refactor-iter-types

Conversation

@demosdemon
Copy link
Contributor

This partially refactors some of the iterator types into submodules.

The peek method was added to ReturnableIterator because it will be necessary in a future PR for #1441.

@demosdemon demosdemon marked this pull request as ready for review December 11, 2025 16:25
@demosdemon demosdemon requested a review from rkuris as a code owner December 11, 2025 16:25
/// An iterator over key-value pairs that stops after a specified final key.
#[derive(Debug)]
#[must_use = "iterators are lazy and do nothing unless consumed"]
pub enum FilteredKeyRangeIter<I, K> {
Copy link
Member

Choose a reason for hiding this comment

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

nit: naming

FilteredKeyRangeIter::Unfiltered seems odd. Perhaps MaybeFilteredKeyRangeIter is better?

}
}

impl<I: Iterator<Item = T>, T: KeyValuePair, K: KeyType> Iterator for FilteredKeyRangeIter<I, K> {
Copy link
Member

Choose a reason for hiding this comment

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

Should also impl FusedIterator since this code will also fuse the underlying iterator.

Probably should document that it will fuse it as well.


impl<I: Iterator> ReturnableIteratorExt for I {}

/// Similar to a peekable iterator. In addition to being able to peek at the
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I'm following what this does that Peekable's blanket implementation does not do., except it's a little less flexible. Peekable keeps a peeked: Option<Option<I::Item>> inside it, but also supports stuff like peek_mut which can be used to implement return_item.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also supports stuff like peek_mut which can be used to implement return_item.

peek_mut does not do what I need. If it returned &mut Option<I::Item>, that would be sufficient. However, it returns Option<&mut I::Item> which is not the same thing.

Peekable provides no way for us to push the item back to the front of the iterator.

This is similar to using VecDeque to push_front but with a slot for only one element.

@demosdemon demosdemon marked this pull request as draft December 15, 2025 20:16
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.

2 participants