What `.iter().map()` does than simple loop don't?

Are there any rationals to prefer nested .iter().map() instead of a bunch of simple loops.

What .iter().map() does than simple loop don't?

I’m facing this code and think separated loops would make it more readable:

let (pdevice, queue_family_index) = unsafe {
    pdevices
        .iter()
        .map(|pdevice| {
            instance
                .get_physical_device_queue_family_properties(*pdevice)
                .iter()
                .enumerate()
                .filter_map(|(index, ref info)| {
                    let supports_graphic_and_surface =
                        info.queue_flags.contains(vk::QueueFlags::GRAPHICS)
                            && surface_loader.get_physical_device_surface_support(
                                *pdevice,
                                index as u32,
                                surface,
                            );
                    if supports_graphic_and_surface {
                        Some((*pdevice, index))
                    } else {
                        None
                    }
                })
                .next()
        })
        .filter_map(|v| v)
        .next()
        .expect("Couldn't find suitable device.")
};

Context: It's a Vulkan code trying to find a physical device from its capability to manage graphic queue.

As you guess, I'm new to rust, but I think short simple loops are easier to read and maintain on the long run, even in other language (C, C++, Python in my situation).

Is this waterfall style encouraged? Is there rationals for this? Am I missing something?

A big thanks in advance. Rust is pretty cool to work with. It's crazy to be as ninja-ish as Python but actually writing system code…

1 Like

The code you posted is indeed a bit excessive, and the use of next() is weird.

1 Like

Without much study (could be wrong) it looks like find would be better that next.

What iterator does:

  • Iterator returns a meaningful value.
    • With loop, you cannot get a direct value and you will need temporary variable outside the loop.
  • Iterator is easy to chain modifications.
    • With loop, you will need temporary variables or deeply nested function calls.
      (For example, let a = foo(v); let b = bar(a); let c = baz(b); ... or somefunction(...(baz(bar(foo(v))))...).
    • You can easily insert modification between existing modifications.

What iterator guarantees by not doing:

  • You cannot use arbitrary too-powerful control flow (break, return, continue, etc.) without explicit iterator combinator.
    • This makes expressions easier to read.
    • Examples:
      • You are sure .map() doesn't break or continue or return, so the number of element won't be changed here.
      • You are sure .filter() reduces the number of elements, but neither increases the number nor modifies each iterated value.

Legacy programming languages has too powerful goto statement, but people prefer weaker alternatives (if, while, for, etc.) because they are not too powerful (than goto).
Such restrictions are usually helpful for readers to understand the code.

I think iterator and its combinators are such weaker and easier-to-understand alternatives of loops.
Developers need to use many combinators .map(), .filter(), .scan(), ... instead of single loop, but they will be easier to read because they let developer do only some allowed operations.

12 Likes

iter.filter_map(closure).next() is equivalent to iter.find_map(closure). filter_map an iterator of Options is equivalent to flat_map.

let (pdevice, queue_family_index) = unsafe {
    pdevices
        .iter()
        .flat_map(|pdevice| {
            instance
                .get_physical_device_queue_family_properties(*pdevice)
                .iter()
                .enumerate()
                .find_map(|(index, ref info)| {
                        if info.queue_flags.contains(vk::QueueFlags::GRAPHICS)
                            && surface_loader.get_physical_device_surface_support(
                                *pdevice,
                                index as u32,
                                surface,
                            ) {
                            Some((*pdevice, index))
                        } else { None }
                })
        })
        .next()
        .expect("Couldn't find suitable device.")
};

P.S. There is a new code review category you can use.

1 Like

There are a bunch of dependencies to the snippet, so I can't test it, but it seems like this should be equivalent:

let (pdevice, queue_family_index, _info) = unsafe {
    pdevices
        .iter()
        .copied()
        .flat_map(|pdevice| {
            instance
                .get_physical_device_queue_family_properties(pdevice)
                .into_iter()
                .enumerate()
                .map(move |(idx, info)| (pdevice, idx as u32, info))
        })
        .find(|(pdevice, idx, info)| {
            info.queue_flags.contains(vk::QueueFlags::GRAPHICS)
                && surface_loader.get_physical_device_surface_support(
                    *pdevice,
                    *idx,
                    surface,
                )
        })
        .expect("Couldn't find suitable device.")
};

I would probably also suggest decreasing the scope of that unsafe block.

1 Like

First, a big thanks to all your answers. :heart:

So it looks like filter/map/find/closure is the way to go. Damn! Looks like I'll have to love them.

I consider the problem solved: The loop could be better written, but not with simple loops.

I will mark @lo48576 answer as solving the problem because it explains the rationals behind filter/map/find/closure idiom and practically convinced me, but most code you've all posted will help me a lot so thanks a lot to you all!

Last curious question: Beyond the language semantic itself, is there any optimization topic iterator involves?

P.S. There is a new code review category you can use.

Thanks, I've edited my original post.

Rust performs by default boundary checks if you index arrays and stuff, e.g. vec[4]. In loops this can become costly. In simple loops the optimizer can figure out when out-of-bounds-checks can safely be removed, e.g.

let v = vec![1, 2, 3, 5];

for idx in 0..v.len() {
    println!("Index times element: {}", idx * v[idx]);
}

But if loops get complicated the optimizer might fail to see such cases and leave them in the code.

On the other hand, the iterators ensure that they don't access memory out of bounds. Hence, there are no out-of-bounds-checks by default.

In the end, iterators might give some performance advantage but it heavily depends, so there is no other way then measuring :wink:.


The example could also be written as:

vec![1, 2, 3, 5].into_iter()
    .enumerate()
    .map(|(idx, value)| idx * value)
    .for_each(|res| println!("Index times element: {}", res));
2 Likes

Iterators are not more and not less than loops, and you should choose between them based on good taste instead of dogmatic rules.

5 Likes

I'll add that it's not necessarily one or the other. It can be very nice to have an ordinary for loop where the body uses iterators to do something. (Consider extracting a function, for example, so the outer for loop could just return the value once it finds it, and panic!("Couldn't find suitable device.") after the loop. But then it could still use iterators to look for a suitable value for the given device.)

Maybe you could try using clippy, it lints and recommends some of the stuff here and beginners to rust may learn some new API such as using find() instead of filter().next() without going through all the documentation, sometimes not just beginners but seasoned programmers may forget some of those, such as adding return in the last statement.