flatten_ok

[vadixidav]

Proposal

I propose adding the following (without the impl Iterator, just to demonstrate):

    fn flatten_ok<A, B, E>(self) -> impl Iterator<Item=Result<B, E>>
        where Self: Iterator<Item = Result<A, E>>, A: IntoIterator<Item = B>
    {...}

flatten_ok operates on an iterator of Result<A, E> where A: IntoIterator<Item = B>. If the iterator produces Ok, the value is flattened. If the iterator produces Err, the value is produced exactly as-is.

Why add this?

Several others have been trying to solve this exact problem:

• https://paulkernfeld.com/2018/11/03/flatten-nested-iterator-result.html
• https://stackoverflow.com/questions/59852161/how-to-handle-result-in-flat-map

I also encountered this problem and was unable to find a combinator for this purpose, which was surprising to me.

Potential way to implement

It could be implemented this way:

fn flatten_ok<A, B, E>(self) -> impl Iterator<Item=Result<B, E>>
where Self: Iterator<Item = Result<A, E>>, A: IntoIterator<Item = B>
{
    self.flat_map(|res| Either::from(res)
        .map_right(|ok| ok.into_iter().map(Ok))
        .map_left(Err)
        .map_left(std::iter::once)
    )
}

Of course, a struct could be created within itertools to implement the Iterator so there is no external API dependency on either, and so we can still specify a concrete return type, but the above would approximately the implementation used and does work if this signature (with impl Iterator) is acceptable.

Questions

• Is there another and more clear way to implement this that others are missing? If so, please let me know.
• Should this be proposed to libcore first?

[vadixidav]

If this proposal (with or without impl Iterator) is acceptable, I can send in a PR. Just let me know if its good.

[jswrenn]

Yes, this proposal seems acceptable! Note:

• Unfortunately, using impl trait isn't an option on trait methods (which is what Itertools provides). You'll need to define a concrete FlattenOk return type.
• We already depend on Either, so no issue there!

You're welcome to propose this to libcore first, but you don't have to. I'm happy to accept a PR for this here. (The absolute worst case scenario is that we might, someday, need to remove it from Itertools if it gets upstreamed into libcore. That's fine — we do make breaking changes from time to time anyways — and the underlying cause for this breakage may soon be addressed by a language RFC.)

[vadixidav]

@jswrenn Excellent! I will get to work on this later today after work.

[SkiFire13]

I would be wary of using Either with iterators since it adds an additional discriminant check for each next call.

In my opinion a faster implementation would be taking FlattenCompat from the stdlib and special casing it for Result. In particular when iter yields an Err I would not store any iterator in frontiter or backiter because that iterator would already be exhausted.

[mpizenberg]

I've faced the same issue. For me, a solution without Either was based on scan: https://users.rust-lang.org/t/handling-errors-inside-flat-map-iterator/55238/11

[vadixidav]

@mpizenberg I am going to start working on a PR for this today. I will take a look at the scan version too. It sounds like, for efficiency reasons, it may be best to just do it from scratch though, but I can compare.