Examples of bad places to use iterators?

In the Common newbie mistakes or bad practices thread it was mentioned that there are places where iterators are the right tool for the job and places where you should use indexing instead.

Do you have any examples where the use of iterators made something less readable or harder to reason about than its imperative counterpart?

I know I've written some particularly atrocious train wrecks iterator chains in my time and when you combine it with other monad-like types like Option, Result, and Future, you can create some real monsters, but I can't think of any examples that won't come across as contrived.

I often find it useful to store indices rather than iterators when writing a lexer or parser, especially with lookahead. Here's a simple example where I used a combination of iterators and indices:
Unicode tokenization - #12 by mbrubeck

More generally, indices can be useful when you need to mutate a collection while iterating over it, or when you need to move backward and forward by varying amounts, or when you want the same struct to store both the collection and the iteration state without becoming a dreaded self-referential struct.


This should be the other way round, right?

Right. Fixed, thanks.

Quickly looking through the places I have used for loops instead of iterators, since I'm generally very iterator-heavy:

  • Sometimes, when mutable references are involved, it's not easily possible to use an iterator, at least without "streaming iterators". The linked code (in reduced form below) modifies values in a slice in a cyclical fashion, re-borrowing the slice at the end of a cycle. This can't be encapsulated in Iterator, because Iterator can only return references with the same lifetime as the implementor, and also mutable references are not Clonable. With non-lexical lifetimes, however, you can re-borrow the original slice from inside a loop!
let mut foo_iter = foos.iter_mut();
for bar in bars {
    let foo = match foo_iter.next() {
        Some(foo) => foo,
        None => {
            foo_iter = foos.iter_mut();
            foo_iter.next().unwrap_or_else(|| unreachable!())
    *foo += bar;

Iterators can be super convoluted in graphics processing when you need to access multiple lines. For example, single-pass 9-pixel blur can be implemented easily and efficiently using a single for loop with an accumulator and +1/-1, or it can be an awkward zip of multiple windowing iterators.


I agree and I suppose that's similar to my case above, in that Iterators are 1-dimensional abstraction, which makes 2d operations (like on pixels, or in my case, a polyphase filter). Both can be done with matrices, but I don't think I've seen any that work like or are FP quite like Iterators..

I was looking for a snippet of code to use as an example and stumbled across your post from 2016 :sweat_smile:

It took me a bit of thinking, but I've come up with a functional version which does roughly the same amount of work as the imperative version while being only mildly unreadable.

pub fn functional_blur(input: &Matrix) -> Matrix {
    assert!(input.width >= 3);
    assert!(input.height >= 3);

    // Stash away the top and bottom rows so they can be
    // directly copied across later
    let mut rows = input.rows();
    let first_row = rows.next().unwrap();
    let last_row = rows.next_back().unwrap();

    let top_row = input.rows();
    let middle_row = input.rows().skip(1);
    let bottom_row = input.rows().skip(2);

    let blurred_elements = top_row
        .flat_map(|((top, middle), bottom)| blur_rows(top, middle, bottom));

    let elements: Vec<f32> = first_row

    Matrix::new_row_major(elements, input.width, input.height)

fn blur_rows<'a>(
    top_row: &'a [f32],
    middle_row: &'a [f32],
    bottom_row: &'a [f32],
) -> impl Iterator<Item = f32> + 'a {
    // stash away the left-most and right-most elements so they can be copied across directly.
    let &first = middle_row.first().unwrap();
    let &last = middle_row.last().unwrap();

    // Get the top, middle, and bottom row of our 3x3 sub-matrix so they can be
    // averaged.
    let top_window = top_row.windows(3);
    let middle_window = middle_row.windows(3);
    let bottom_window = bottom_row.windows(3);

    // slide the 3x3 window across our middle row so we can get the average
    // of everything except the left-most and right-most elements.
    let averages = top_window
        .map(|((top, middle), bottom)| top.iter().chain(middle).chain(bottom).sum::<f32>() / 9.0);



Meanwhile, my imperative version is a couple loops and some copy/paste.

/// A simple blur implemented by averaging values in the 3x3 area around a
/// pixel.
pub fn imperative_blur(input: &Matrix) -> Matrix {
    assert!(input.width >= 3);
    assert!(input.height >= 3);

    // allocate our output matrix, copying from the input so
    // we don't need to worry about the edge cases.
    let mut output = input.clone();

    for y in 1..(input.height - 1) {
        for x in 1..(input.width - 1) {
            let mut pixel_value = 0.0;

            pixel_value += input[[x - 1, y - 1]];
            pixel_value += input[[x, y - 1]];
            pixel_value += input[[x + 1, y - 1]];

            pixel_value += input[[x - 1, y]];
            pixel_value += input[[x, y]];
            pixel_value += input[[x + 1, y]];

            pixel_value += input[[x - 1, y + 1]];
            pixel_value += input[[x, y + 1]];
            pixel_value += input[[x + 1, y + 1]];

            output[[x, y]] = pixel_value / 9.0;

1 Like

You'd also need extra cases for first/last pixel of each row, and a bit of gymnastics with mutable accumulators in closures to avoid calculating sum for every pixel (to move from [A,B,C] to [B,C,D] you only need += D - A, and that nicely works for any width). In imperative version that's straightforward and even makes the inner loop a couple of lines shorter. With iterators I remember struggling with this when I was a noob, and just wrote a generic loop instead.

Yeah, figuring out how to handle the first/last pixel in each row was annoying. My iterator version's blur_rows() function ended up copying doing a funky iter::once(first).chain(averages).chain(iter::once(last)) to create an iterator with the correct values, then I added a test to make sure both implementations behaved identically.

Did this have much of an effect on performance? Looking at the code, I would have thought sum = sum + D - A will have the same cost as sum = B + C + D when doing a 3x3 blur.

Caution: this algorithm is only correct on integers. On floats, it accumulates error, because adding a small number to a large one rounds off some of the bits of the small number's mantissa.

(There are algorithms to do better, but I don't know enough about them to recommend one, and they are probably not superior to the straightforward approach of summing B + C + D from scratch for this small size.)


I agree that 1D per-pixel iterators for 2D images are awkward. But it's not clear to me that it's exactly the iterators that are the problem there.

For example, you could do 1D blurs with a slice's windows iterator, if you wanted. An image crate could analogously provide iterators over windows or chunks of the image in 2D, which could then be processed with iterators (zipping, mapping, etc).

1 Like

Now that you mention it, ndarray actually seems to support 2d windows.. though only with shared references.

This thread made me realize I can use Cell::from_mut to accomplish this:

fn example<T>(foos: &mut [T], bars: impl IntoIterator<Item = T>)
where T: core::ops::Add<Output = T> + Copy {
    let foo_cells = core::cell::Cell::from_mut(foos).as_slice_of_cells();
    let foo_cyclical_iter = foo_cells.iter().cycle();
    foo_cyclical_iter.zip(bars).for_each(|(foo, bar)| foo.set(foo.get() + bar));

Playground Link


Slices also only have windows, no windows_mut -- it's be a mutable aliasing violation with a normal (non-streaming/lending) iterator.

1 Like

When you mix both peekable and by_ref you can easily shoot yourself in the foot. I did that myself back then, not sure why it is possible with the API.

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.