CODEWITHSUNDEEP
rustrefactoringreadability
GMA
GMA

Reputation: 6096

How to avoid nested chains of "if let"?

I'm wading through a codebase full of code like this:

if let Some(i) = func1() {
    if let Some(j) = func2(i) {
        if let Some(k) = func3(j) {
            if let Some(result) = func4(k) {
                // Do something with result
            } else {
                println!("func 4 returned None");
            }
        } else {
            println!("func 3 returned None");
        }
    } else {
        println!("func 2 returned None");
    }
} else {
    println!("func 1 returned None");
}

That's a stupid, simplified example, but the general pattern is that:

The problem, of course, is that the above code is an ugly and unreadable. It gets even uglier when you substitute i, func1 etc. with variable/function names that actually mean something in the real code, and many examples in my real codebase have far more than four nested if lets. It's an example of the arrow anti-pattern, it completely fails the squint test, and it's confusing how the error messages appear in reverse order to the functions which can cause them.

Is there really not a better way to do this? I want to refactor the above into something that has a cleaner, flatter structure where everything appears in a sensible order. if let chaining might help but it doesn't look like that feature is available in Rust yet. I thought maybe I could clean things up by using ? and/or extracting some helper functions, but I couldn't get it to work and I'd rather not extract a ton of new functions all over the place if I can avoid it.

Here's the best I could come up with:

let i : u64;
let j : u64;
let k : u64;
let result : u64;

if let Some(_i) = func1() {
    i = _i;
} else {
   println!("func 1 returned None");
   return;
}
if let Some(_j) = func2(i) {
    j = _j;
} else {
   println!("func 2 returned None");
   return;
}
if let Some(_k) = func3(j) {
    k = _k;
} else {
   println!("func 3 returned None");
   return;
}
if let Some(_result) = func3(k) {
    result = _result;
} else {
   println!("func 4 returned None");
   return;
}


// Do something with result

But this still feels very long and verbose, and I don't like how I'm introducing these extra variables _i, _j etc.

Is there something I'm not seeing here? What's the simplest and cleanest way to write what I want to write?

Upvotes: 36

Views: 8707

Answers (6)

Gerardo Marset
Gerardo Marset

Reputation: 843

An alternative while we wait for if-let-chains to be stabilized:

if let (Some(foo), Some(bar)) = (try_get_foo(), try_get_bar()) {
    foo(bar);
}

There's also the if_chain crate:

fn get_argument_fmtstr_parts(expr: &Expr) -> Option<(InternedString, usize)> {
    if_chain! {
        if let ExprAddrOf(_, ref expr) = expr.node; // &["…", "…", …]
        if let ExprArray(ref exprs) = expr.node;
        if let Some(expr) = exprs.last();
        if let ExprLit(ref lit) = expr.node;
        if let LitKind::Str(ref lit, _) = lit.node;
        then {
            return Some((lit.as_str(), exprs.len()));
        }
    }
    None
}

Upvotes: 0

John Kugelman
John Kugelman

Reputation: 361585

You can use let-else statements, a feature which was added to stable rust in version 1.65.

RFC 3137

Introduce a new let PATTERN: TYPE = EXPRESSION else DIVERGING_BLOCK; construct (informally called a let-else statement), the counterpart of if-let expressions.

If the pattern match from the assigned expression succeeds, its bindings are introduced into the surrounding scope. If it does not succeed, it must diverge (return !, e.g. return or break).

With this feature you can write:

let Some(i) = func1() else {
    println!("func 1 returned None");
    return;
};
let Some(j) = func2(i) else {
    println!("func 2 returned None");
    return;
};
let Some(k) = func3(j) else {
    println!("func 3 returned None");
    return;
};
let Some(result) = func3(k) else {
    println!("func 4 returned None");
    return;
};

If you wanted to try it on an older version of rust, you would have to enable the unstable feature:

#![feature(let_else)]

Upvotes: 35

mikysett
mikysett

Reputation: 11

I had a very similar issue and the solution of @Caesar helped me a lot.

I would add in my case I need the results of all the functions involved, and I've modified that code to achieve that in this way:

let Some((first, Some(second)) = a.func().and_then(|first| Some((first, b.func(second))) {
    result += c.func(first, second);
}

The down side is it could end up by having a lot of nested Some for long chains, but in my case I only have two levels and this solution seems to be quite efficient.

PS: cargo clippy suggested to use map instead of and_then, which is indeed shorter and cleaner in this case :)

Upvotes: 1

Chayim Friedman
Chayim Friedman

Reputation: 70910

A slightly better version of the if let ... else { return } can be used:

let i = if let Some(i) = func1() { i } else {
   println!("func 1 returned None");
   return;
};
let j = if let Some(j) = func2(i) { j } else {
   println!("func 2 returned None");
   return;
};
let k = if let Some(k) = func3(j) { k } else {
   println!("func 3 returned None");
   return;
};
let result = if let Some(result) = func3(k) { result } else {
   println!("func 4 returned None");
   return;
};

Upvotes: 4

Caesar
Caesar

Reputation: 8484

I'd like to put two more things on your list of "eventually look into" things: The simple Option::and_then:

let result = func1().and_then(func2).and_then(func3).and_then(func4);
match result {
  Some(result) => …,
  None => …,
}

And the slightly more tricky, but incredibly convenient anyhow::Context:

use anyhow::Context;
let j = func2(i).context("Func2 failed")?;

Upvotes: 8

cameron1024
cameron1024

Reputation: 10136

If-let chaining will make this a lot nicer, but for now (assuming you don't want to use nightly), it's possible a slight refactor could help. For example, pulling all but the last call of the chain into its own function allows you to use the ? operator:

fn get_result() -> Option<u64> {
  let i = func1()?;
  let j = func2(i)?;
  let k = func3(j)?;
  func3(k)
}

fn main() {
  if let Some(result) = get_result() {
    // do something
  }
}

If you need more fine-grained control over the error cases, you could return a Result instead:

enum Error {
  Func1,
  Func2,
  Func3,
  Func4,
}

fn get_result() -> Result<i64, Error> {
  let i = func1().ok_or(Error::Func1)?;
  let j = func2(i).ok_or(Error::Func2)?;
  let k = func3(j).ok_or(Error::Func3)?;
  func4(k).ok_or(Error::Func4)
}

fn main() {
  use Error::*;
  match get_result() {
    Ok(result) => {},
    Err(Func1) => {},
    // ...
  }
}

Upvotes: 17

Related Questions