Filter a HashMap & propagate errors

help
1

I have a hash map of tasks, HashMap<TaskName, Task>, I'd like to filter it based on a predicate. Use HashMap::filter(). OK. Thanks.

But, but, but, stay here a little more please. :slight_smile:

My predicate does not give me a bool, but a Result<bool, SomeError>. I want to propagate any error encountered. Here is it in code:

type TaskName = String;
struct Task { /* Whatever */ }
struct SomeError (/* whatever */);

impl Task {
    fn predicate(&self) -> Result<bool, SomeError> {
        // This one is not a problem, let's say it failed for the example.
        Err(SomeError())
    }
}

fn filter_the_map(tasks: &mut HashMap<TaskName, Task>) -> Result<(), SomeError> {
    tasks.into_iter().filter(|(k, v)| {
        v.predicate()?
    })
    Ok(())
}

This does not work, as the ? shortcut for propagating the error is into the closure, not into the function itself.

tasks
    .map(|(k, v)| v.predicate())  // Iterator<Item = Result<bool, SomeError>>
    .collect::<Result<Vec<Task>, SomeError>>()?  // Vec<Task> or short-return

This one does not work much: error handling is OK, filtering is OK, but I have a Vec<Task> at the end, instead of modifying my HashMap<TaskName, Task> in any way.

2

The types don't check out in your examples at all, so I don't understand what you are actually trying to do with the errors.

  1. If you don't want a vector but a hash map, then why don't you collect into a HashMap instead of a Vec?
  2. Your "this does not work" example doesn't modify the hash map, either. It turns the reference into an iterator (over (&K, &mut V) pairs) which it then filters and discards. As a result, you are not actually doing anything with the map.
3
  1. What should be the type, after collecting? Result<_, SomeError> for sure, as I want to propagates the error; Result<HashMap<_, _>, SomeError> maybe, I imagine… but I do need both the predicate's bool to filter & the (TaskName, Task) to rebuild the HashMap.
  2. Yes indeed. Maybe the good method to use would be retain(): tasks.retain(|(k, v)| { v.predicate()? }), which "does not work" for the same reason.
4

Does this help?

5

Yes, it matches the objective. Do you think at something more succinct using built-in function? try_retain() does not exist, but it might have done what I'm looking for.

6

I don't think there's anything better. What's wrong with this approach? What's not "built-in" enough about it? It only uses the standard library.

Sometimes, not everything is pre-written for you, and you actually have to come up some sort of solution yourself. Not like this is a hard problem to solve.

7

A try_retain logic is hard to reason about though.

What happens when your predicates errors midway?

  1. Just stop the filtering process and leave some entries that should have been filtered unaffected. (That's what happens if ? worked through closure magically.)
  2. Discard all entries. (That's what happens if you use a .into_iter().filter().collect() approach)
  3. Just report the first error and filter all entries anyway. (That's what @paramagnetic 's code doing, it's still unclear whether error means filtered or not.)
2 Likes
8

A drawback of using retain is that there is no way of exiting early, and the original map is changed even when an error occurs. This can be avoided by returning a new HashMap instead (playground):

fn filter_the_map(tasks: &HashMap<TaskName, Task>) -> Result<HashMap<&TaskName, &Task>, SomeError> {
    tasks.into_iter().filter_map(|(k, v)| {
        match v.predicate() {
            Ok(true) => Some(Ok((k,v))),
            Ok(false) => None,
            Err(e) => Some(Err(e))
        }
    }).collect()
}

If you do not need the original map in case of an error, you can pass the map by value to the filter functions instead.

Unfortunately, there is no try_filter on iterators (or try_retain on HashMap). Instead of the above, i often find a chain of .map(...).filter_map(Result::transpose) to be more readable:

fn filter_the_map(tasks: &HashMap<TaskName, Task>) -> Result<HashMap<&TaskName, &Task>, SomeError> {
    tasks.into_iter().map(|(k, v)| {
        Ok(v.predicate()?.then_some((k,v)))
    })
    .filter_map(Result::transpose)
    .collect()
}
9

This approach will not return anything (from the original HashMap) unless v.predicate() always returns Ok. I doubt that's ever the desired behaviour.

10

@maarten Nice idea of transforming the bool to Option + filter_map. Just for the tip, std::mem::replace() may help there for the pattern “build a new HashMap & return it on its &mut reference”.

@zirconium-n Very interesting thoughts about the difficulties of try_reclaim.

Thank you everyone.

11

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.