A misunderstanding of map() that cost me a day

1

Once I figured out what was going on, it was obvious, but it took me a whole day. I'm hoping this write-up will save someone else that day.

I'm not a total idiot although some would disagree. The symptom was that some of the elements in a HashMap were set correctly, but others weren't. It took me most of the day to isolate the problem to the statement illustrated below and an hour of staring to see my mistake.

I know that map() is lazy, so I use for-loops when I just have side effects. However, it seemed reasonable to use fluent style for this case. (The actual code has more steps.)

let bar = foos // A HashMap
     .values_mut()
     .map(|foo| {
         println!("foo.id");
         foo.set_one_or_two(x); // x is a boolean so not every foo gets set to 1
         foo })
    .find(|foo| foo.is_one())
     //.last()
     .unwrap()
)};

When I run with find() commented out and last() not, all values of the HashMap get printed. When last() is commented out but find() is not, I only see some of them. In my head I had map() feeding all elements into find(). Doing that would violate the lazy nature of map(). Instead, find() asks for an element from from map() until it gets a true, then it stops asking. Doh!

2 Likes
2

Yep. Most of the time, this is a feature. :slight_smile:

I've come to look at iterator chains and first see what is driving/consuming them, (for, collect, etc) and then check for the "short circuit" operations (like find, take, sometimes fuse, etc). If these are missing, I know that the return type is still an iterator. I find that hover-over type annotation in an IDE (vscode) is really helpful to see this.

A related post I saw on twitter yesterday with a similar trap:

https://esimmler.com/why-arent-my-rust-threads-running/

Minor nit: it's not map() that's lazy, but the entire iterator chain. It is an iterator method that's commonly used to do work in such a chain, though, so when that work is not getting done, it seems fair to call it out as lazy.

3

I can see myself making this exact same mistake. Thanks for sharing!

2 Likes
4

I ended up doing the work in a for-loop. It's ugly and has mutable temporaries, but it avoids traversing the hash map twice.

5

If you want to keep the same style, apply the map to all elements, and remember the first one to match, try something with iter.map(..).fold(..) to both drive the iterator and keep state across items for what you want to return at the end.

6

That's a good idea, so I implemented it. Unfortunately, due to the actual code being somewhat more complicated, I found the for-loop version easier to read.

1 Like
7

One of the great things about rust is that you can just use a for loop, and you shouldn't feel bad about doing so. And with safe code guaranteed data-race-free, most of the mutability downsides go away.

2 Likes