Is it idiomatic to use an immediately called closure to handle errors?

help
1

For example:

let id = (|| {
    (kmp_path.next_nodes.len() == 1).then_some(())?;
    let next_node = world.entity(*kmp_path.next_nodes.iter().next()?);

    (next_node.get::<KmpPathNode>()?.prev_nodes.len() == 1).then_some(())?;
    (next_node.get::<PathOverallStart>().is_none()).then_some(())?;

    let id = **next_node.get::<OrderId>().unwrap();
    Some(id as u8)
})()
.unwrap_or(0xff)

This is the most concise way I have found to write this logic out. I tried nested if statements but it got messy quickly.
The bool expressions with then_some(())? do seem dodgy, but at the same time it is an efficient way of writing it as opposed to

if kmp_path.next_nodes.len() != 1 {
    return None;
}

What do you think?

2 Likes
2

I've used this technique many times. I've recommended it, I've seen other people recommend it.

I'd even tried to put it in the unofficial patterns guide at one time, didn't find the time:

1 Like
3

Until try_blocks are stabilized, this is probably as good as we get to emulate them.

8 Likes
4

Speaking generally, I prefer making ? applicable in a more structured manner. Like making that closure a method on whatever kmp_path is.[1]

let id = kmp_path.next_order_id(&world).unwrap_or(0xff);

// Or even more granular, like a method for your len == 1 checks,
// that you could make use of in `next_order_id`.

Then months from now you don't have to squint at some potentially dodgy to read code, and can instead read the hopefully descriptive method name to get a summary of where the id is coming from.

Anyway, you're basically emulating try blocks. I'm not sure how idiomatic it is or isn't. I agree it's the closest we've got to try blocks on stable. I've seen it for robust error handling, though I prefer From<(ErrTy, MoreContext)> for that.

I guess in summary I prefer encapsulating the dense but mechanical blocks that tend to want try blocks.

Eh, after playing around awhile, it's fine. You could wrap it in a macro if it bothers you. But there are some other alternatives.

With literals you can write

let 1 = kmp_path.next_nodes.len() else { return None };
// Similar to this but rustfmt lets you one-line it:
// if ! matches!(kmp_path.next_nodes.len(), 1) { return None };
(Probably exactly what you don't want if writing short and fast is your top concern, but sometimes I write 
let () = returns_option_unit_or_result_unit_err()?;

for the sake of clarity.)

  1. AFAICT it doesn't capture anything. ↩ī¸Ž

3 Likes
5

Personally I would lean on the "unidiomatic" side. I've certainly written this pattern a few times myself, and I've seen it a few times in other people's code, but it just looks weird, and requires a bit too much effort for the value it provides. I've used it more often in C++ code. C++ isn't expression-based like Rust is, so immediately called closures are the only way to create scoped mutability (i.e. temporarily mutate a value within a block, and assign it to a const variable afterwards). But Rust almost never requires such tricks, so I prefer to avoid it in this edge case as well. Either extract that block into a proper function, or use Option/Result combinators.

5 Likes
6

Note in the context of these quotes that Rust's optimizer is very good at detecting a function with a single caller and inlining the body for you. It's thus not harmful to split into lots of small functions; indeed, it can improve compile times, since there's a few algorithms that are supra-linear in the size of the function body.

2 Likes
7

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.