-
Notifications
You must be signed in to change notification settings - Fork 340
Fix pad_using bug producing items when it shouldn't
#1079
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
base: master
Are you sure you want to change the base?
Changes from all commits
625c2b9
979d7a7
8e8b98a
ee3022a
5524abc
537d094
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| use crate::size_hint; | ||
| use std::iter::{Fuse, FusedIterator}; | ||
|
|
||
| /// An iterator adaptor that pads a sequence to a minimum length by filling | ||
|
|
@@ -11,28 +10,36 @@ use std::iter::{Fuse, FusedIterator}; | |
| #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] | ||
| pub struct PadUsing<I, F> { | ||
| iter: Fuse<I>, | ||
| min: usize, | ||
| pos: usize, | ||
| elements_from_next: usize, | ||
| elements_from_next_back: usize, | ||
| elements_required: usize, | ||
| filler: F, | ||
| } | ||
|
|
||
| impl<I, F> std::fmt::Debug for PadUsing<I, F> | ||
| where | ||
| I: std::fmt::Debug, | ||
| { | ||
| debug_fmt_fields!(PadUsing, iter, min, pos); | ||
| debug_fmt_fields!( | ||
| PadUsing, | ||
| iter, | ||
| elements_from_next, | ||
| elements_from_next_back, | ||
| elements_required | ||
| ); | ||
| } | ||
|
|
||
| /// Create a new `PadUsing` iterator. | ||
| pub fn pad_using<I, F>(iter: I, min: usize, filler: F) -> PadUsing<I, F> | ||
| pub fn pad_using<I, F>(iter: I, elements_required: usize, filler: F) -> PadUsing<I, F> | ||
| where | ||
| I: Iterator, | ||
| F: FnMut(usize) -> I::Item, | ||
| { | ||
| PadUsing { | ||
| iter: iter.fuse(), | ||
| min, | ||
| pos: 0, | ||
| elements_from_next: 0, | ||
| elements_from_next_back: 0, | ||
| elements_required, | ||
| filler, | ||
| } | ||
| } | ||
|
|
@@ -44,40 +51,35 @@ where | |
| { | ||
| type Item = I::Item; | ||
|
|
||
| #[inline] | ||
| fn next(&mut self) -> Option<Self::Item> { | ||
| match self.iter.next() { | ||
| None => { | ||
| if self.pos < self.min { | ||
| let e = Some((self.filler)(self.pos)); | ||
| self.pos += 1; | ||
| e | ||
| } else { | ||
| None | ||
| } | ||
| } | ||
| e => { | ||
| self.pos += 1; | ||
| e | ||
| } | ||
| let total_consumed = self.elements_from_next + self.elements_from_next_back; | ||
|
|
||
| if total_consumed >= self.elements_required { | ||
| self.iter.next() | ||
| } else if let Some(e) = self.iter.next() { | ||
| self.elements_from_next += 1; | ||
| Some(e) | ||
| } else { | ||
| let e = (self.filler)(self.elements_from_next); | ||
| self.elements_from_next += 1; | ||
| Some(e) | ||
| } | ||
| } | ||
|
|
||
| fn size_hint(&self) -> (usize, Option<usize>) { | ||
| let tail = self.min.saturating_sub(self.pos); | ||
| size_hint::max(self.iter.size_hint(), (tail, Some(tail))) | ||
| } | ||
| let total_consumed = self.elements_from_next + self.elements_from_next_back; | ||
|
|
||
| if total_consumed >= self.elements_required { | ||
| return self.iter.size_hint(); | ||
| } | ||
|
Comment on lines
+72
to
+74
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice and simple. Don't overcount |
||
|
|
||
| let elements_remaining = self.elements_required - total_consumed; | ||
| let (low, high) = self.iter.size_hint(); | ||
|
|
||
| fn fold<B, G>(self, mut init: B, mut f: G) -> B | ||
| where | ||
| G: FnMut(B, Self::Item) -> B, | ||
| { | ||
| let mut pos = self.pos; | ||
| init = self.iter.fold(init, |acc, item| { | ||
| pos += 1; | ||
| f(acc, item) | ||
| }); | ||
| (pos..self.min).map(self.filler).fold(init, f) | ||
| let lower_bound = low.max(elements_remaining); | ||
| let upper_bound = high.map(|h| h.max(elements_remaining)); | ||
|
|
||
| (lower_bound, upper_bound) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -87,25 +89,20 @@ where | |
| F: FnMut(usize) -> I::Item, | ||
| { | ||
| fn next_back(&mut self) -> Option<Self::Item> { | ||
| if self.min == 0 { | ||
| self.iter.next_back() | ||
| } else if self.iter.len() >= self.min { | ||
| self.min -= 1; | ||
| self.iter.next_back() | ||
| } else { | ||
| self.min -= 1; | ||
| Some((self.filler)(self.min)) | ||
| let total_consumed = self.elements_from_next + self.elements_from_next_back; | ||
|
|
||
| if total_consumed >= self.elements_required { | ||
| return self.iter.next_back(); | ||
| } | ||
| } | ||
|
|
||
| fn rfold<B, G>(self, mut init: B, mut f: G) -> B | ||
| where | ||
| G: FnMut(B, Self::Item) -> B, | ||
| { | ||
| init = (self.iter.len()..self.min) | ||
| .map(self.filler) | ||
| .rfold(init, &mut f); | ||
| self.iter.rfold(init, f) | ||
| let index_from_back = self.elements_required - self.elements_from_next_back - 1; | ||
| self.elements_from_next_back += 1; | ||
|
|
||
| if index_from_back >= self.iter.len() { | ||
| Some((self.filler)(index_from_back)) | ||
| } else { | ||
| self.iter.next_back() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please You could even think about incrementing in each branch explicitly (just as you did in
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Please add |
||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Looks very nice imo. It tells
total_consumeddoes not overcountelements_required, which (presumably) avoids overflows.