Implement peeking_fold_while

[mingyli]

Issue

try_fold is capable of folding an iterator until a short-circuiting condition is met. For instance, the short-circuiting example in the docs sums up integers until an integer overflow is encountered. However, try_fold ends up consuming the element that caused the short-circuit (in this case, the element 100).

let a = [10, 20, 30, 100, 40, 50];
let mut it = a.iter();
let sum = it.try_fold(0i8, |acc, &x| acc.checked_add(x).ok_or(acc));
assert_eq!(sum, Err(60));
assert_eq!(it.next(), Some(&40));

Solution

I've implemented a PeekingFoldWhile trait for Peekables and PutBacks with a method peeking_fold_while that does not consume the short-circuiting element. Using it on the above example leads to the following behavior:

use itertools::Itertools;

let mut it = a.iter().peekable();
let sum = it.peeking_fold_while(0i8, |acc, &&x| acc.checked_add(x).ok_or(acc));
assert_eq!(sum, Err(60));
assert_eq!(it.next(), Some(&100));

This is my first attempt at contributing so feedback is welcome 😄

[phimuemue]

This is an interesting idea, and maybe I am missing something, but I suspected that try_fold allows keeping the failure element by specifying an appropriate accumulation function:

let a = [10, 20, 30, 100, 40, 50];
let mut it = a.iter();
let sum = it.try_fold(0i8, |acc, &x| acc.checked_add(x).ok_or((acc, x)));
assert_eq!(sum, Err((60, 100)));

This has the advantage that it works for any Iterator and it is possibly more efficient (Peekable.next must check if there currently is a peeked-element, right).

Could you shed some light on the advantages of peeking_fold_while over the solution via try_fold?

[mingyli]

I actually hadn't thought of your solution and its existence probably eliminates a lot of use cases of peeking_fold_while. I suppose one benefit is that peeking_fold_while enables folding the same iterator multiple times without dropping elements, by not shifting the responsibility of holding onto the short-circuit element to the user. For example,

//       <---->  <----->  <---->
let a = [20, 30, 100, 10, 40, 50];
let mut it = a.iter().peekable();
let sum = it.peeking_fold_while(0i8, |acc, &&x| acc.checked_add(x).ok_or(acc));
assert_eq!(sum, Err(50));
let sum = it.peeking_fold_while(0i8, |acc, &&x| acc.checked_add(x).ok_or(acc));
assert_eq!(sum, Err(110));
let sum = it.peeking_fold_while(0i8, |acc, &&x| acc.checked_add(x).ok_or(acc));
assert_eq!(sum, Ok(90));

whereas using try_fold like above would require unpacking the results to obtain the values 100 and 40, and then passing those into the second and third fold calls. I guess we'd have to determine whether the additional benefit of this interface outweighs polluting itertools with something that is slightly different from something already in the standard library.

[phimuemue]

Thank you for your elaboration. I agree, we should see if it's worth to include it in the crate.

As a sidenote: The use case you gave reminded me of something like group_while, which I could not find anywhere... Does anyone know if a functionality like this exists in Iter or Itertools?

[mingyli]

Given that there is precedence with peeking_take_while is this something we should revisit?

[G-M0N3Y-2503]

However, there are a handful of issues with the current peeking_fold_while:

1. It doesn't use the experimental std::ops::Try which would allow for non-eronious short-circuiting, though this is just for readability.
2. It restricts the implementation for PutBack<I> by unnecessarily constraining it to an immutable reference, where the owned value could be necessary for some.
3. With only a reference, a fold may not be able to be completed only validated, It could need the owned value and the implementation for Peekable<I> currently discards this.

From #979 (comment)

On point 3 It may be useful to cache part of the validation one in the consuming step like peeking_map allowed.