Filter() which is more idiomatic

help
1

If I have an iterator that I need to filter on multiple conditions, is it considered more idiomatic to write it as
iter.filter(cond1).filter(cond2)
or
iter.filter(cond1 && cond2)

2

Assuming you meant to use a closure for the second one so it actually compiles, I don't think it really matters. Use whichever feels right for the situation, or whichever produces a prettier output after rustfmt. I think in the general case the latter reads slightly better (filter-filter can sound a bit confusing), but I can imagine that sometimes the former makes more sense logically.

3 Likes
3

Notwithstanding style, am I right to be concerned that the filter-filter version may not always be optimized to a single predicate? The two filters compose, but is it as optimal?

4

It will most likely be compiled to something similar or the same.

2 Likes
5

Let’s do some “manual inlining” here, step by step.... [relevant definitions are in these two files (here), (here)]

iter.filter(cond1).filter(cond2).next()

Filter::new(iter.filter(cond1), cond2).next()

Filter::new(Filter::new(iter, cond1), cond2).next()

Filter { predicate: cond2, iter: Filter { predicate: cond1, iter }}.next()

Filter { predicate: cond2, iter: Filter { predicate: cond1, iter }}.next()

Filter { predicate: cond1, iter }.find(&mut cond2)

fn check<T>(mut predicate: impl FnMut(&T) -> bool) -> impl FnMut((), T) -> ControlFlow<T> {
    move |(), x| {
        if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    }
}
Filter { predicate: cond1, iter }.try_fold((), check(&mut cond2)).break_value()

fn check<T>(mut predicate: impl FnMut(&T) -> bool) -> impl FnMut((), T) -> ControlFlow<T> {
    move |(), x| {
        if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    }
}
fn filter_try_fold<'a, T, Acc, R: Try<Ok = Acc>>(
    predicate: &'a mut impl FnMut(&T) -> bool,
    mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
    move |acc, item| if predicate(&item) { fold(acc, item) } else { try { acc } }
}
iter.try_fold((), filter_try_fold(&mut cond1, check(&mut cond2))).break_value()

fn filter_try_fold<'a, T, Acc, R: Try<Ok = Acc>>(
    predicate: &'a mut impl FnMut(&T) -> bool,
    mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> R + 'a {
    move |acc, item| if predicate(&item) { fold(acc, item) } else { try { acc } }
}
iter.try_fold((), filter_try_fold(&mut cond1, move |(), x| {
        if (&mut cond2)(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    })).break_value()

iter.try_fold((), move |acc, item| if (&mut cond1)(&item) { (move |(), x| {
        if (&mut cond2)(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    })(acc, item) } else { try { acc } }).break_value()

iter.try_fold((), move |acc, item| if (&mut cond1)(&item) { {
        if (&mut cond2)(&item) { ControlFlow::Break(item) } else { ControlFlow::CONTINUE }
    } } else { try { acc } }).break_value()

(just reformatting in this step)

iter.try_fold((), move |acc, item| {
    if (&mut cond1)(&item) {
        if (&mut cond2)(&item) { ControlFlow::Break(item) } else { ControlFlow::CONTINUE }
    } else { try { acc } }
).break_value()

iter.try_fold((), move |acc, item| {
    if cond1(&item) {
        if cond2(&item) { ControlFlow::Break(item) } else { ControlFlow::CONTINUE }
    } else { try { acc } }
).break_value()

iter.try_fold((), move |acc, item| {
    if cond1(&item) {
        if cond2(&item) { ControlFlow::Break(item) } else { ControlFlow::CONTINUE }
    } else { ControlFlow::Continue(acc) }
).break_value()

iter.try_fold((), move |(), item| {
    if cond1(&item) {
        if cond2(&item) { ControlFlow::Break(item) } else { ControlFlow::Continue(()) }
    } else { ControlFlow::Continue(()) }
).break_value()

which is equivalent to

iter.try_fold((), move |(), item| {
    if cond1(&item) && cond2(&item) {
        ControlFlow::Break(item)
    } else {
        ControlFlow::Continue(())
    }
).break_value()

On the other hand:

iter.filter(|x| cond1(x) && cond2(x)).next()

Filter::new(iter, |x| cond1(x) && cond2(x)).next()

Filter { iter, predicate: |x| cond1(x) && cond2(x) }}.next()

iter.find(&mut |x| cond1(x) && cond2(x))

fn check<T>(mut predicate: impl FnMut(&T) -> bool) -> impl FnMut((), T) -> ControlFlow<T> {
    move |(), x| {
        if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    }
}
iter.try_fold((), check(&mut |x| cond1(x) && cond2(x))).break_value()

iter.try_fold((), move |(), x| {
        if (&mut |x| cond1(x) && cond2(x)))(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    }).break_value()

iter.try_fold((), move |(), x| {
        if cond1(&x) && cond2(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
    }).break_value()

iter.try_fold((), move |(), item| {
    if cond1(&item) && cond2(&item) {
        ControlFlow::Break(item)
    } else {
        ControlFlow::Continue(())
    }
).break_value()

Very likely, similar considerations can show that count, fold and try_fold are equivalent for both iterators. [Well, for count I’m actually not so certain..]

TL;DR, the iterators are “iter.filter(cond1).filter(cond2)” and “iter.filter(cond1 && cond2)” after inlining are about as different as

if cond1 { if cond2 { Some(foo) } else { None } } else { None }

versus

if cond1 && cond2 { Some(foo) } else { None }

I’d also guess that in debug mode the “iter.filter(cond1 && cond2)” version might perform better.

1 Like
6

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.