What is a better way to write this?

[hurricane-socrates]

Thanks in advance. I assume there's a way to avoid the redundant return.

fn get_default_state(doc: &Document) -> String {
    return if let Ok(context) = Context::new(doc) {
        if let Ok(states) = context.evaluate("/scxml/state") {
            if let Some(state) = states.get_nodes_as_vec().first() {
                if let Some(name) = state.get_property("name") {
                    name
                } else {
                    "".to_string()
                }
            } else {
                "".to_string()
            }
        } else {
            "".to_string()
        }
    } else {
        "".to_string()
    };
}

[DanielKeep]

One way to do it:

fn get_state(doc: &Document) -> Option<String> {
    let Ok(context) = Context::new(doc) else { return None };
    let Ok(states) = context.evaluate("/scxml/state") else { return None };
    // ...
    Some(name)
}

fn get_default_state(doc: &Document) -> String {
    get_state(doc).unwrap_or_else(|| "".to_string())
}

Another would be chaining Option::and_then calls, but I find imperative code easier to deal with.

Edit: instead of the let .. else .. pattern, you could also use something like Context::new(doc).ok()?. I used let .. else .. because I started somewhere else and wandered my way into that structure, and didn't think much about it. Y'know, until immediately after hitting "Submit".

[steffahn]

Eventually, let-chains

#![feature(let_chains)]

fn get_default_state(doc: &Document) -> String {
    if let Ok(context) = Context::new(doc)
        && let Ok(states) = context.evaluate("/scxml/state")
        && let Some(state) = states.get_nodes_as_vec().first()
        && let Some(name) = state.get_property("name")
    {
        name
    } else {
        "".to_string()
    }
}

In the mean-time, there's also a crate with a comparably powerful macro, or we can try to use ? operator, with a helper-function:

fn get_default_state(doc: &Document) -> String {
    fn get_or_none(doc: &Document) -> Option<String> {
        let states = Context::new(doc).ok()?.evaluate("/scxml/state").ok()?;
        states.get_nodes_as_vec().first()?.get_property("name")
    }
    get_or_none(doc).unwrap_or_else(|| "".to_string())
}

or an immediately invoked closure

fn get_default_state(doc: &Document) -> String {
    (|| {
        let states = Context::new(doc).ok()?.evaluate("/scxml/state").ok()?;
        states.get_nodes_as_vec().first()?.get_property("name")
    })()
    .unwrap_or_else(|| "".to_string())
}

The "neater" alternative of a try {} block does not exist in stable Rust yet, either.

#![feature(try_blocks)]

fn get_default_state(doc: &Document) -> String {
    let name: Option<_> = try {
        let states = Context::new(doc).ok()?.evaluate("/scxml/state").ok()?;
        states.get_nodes_as_vec().first()?.get_property("name")?
    };
    name.unwrap_or_else(|| "".to_string())
}

Or, if returning the value is the only goal, as in your example, you can use an early-return for the name, and control flow like:

fn get_default_state(doc: &Document) -> String {
    if let Ok(context) = Context::new(doc) {
        if let Ok(states) = context.evaluate("/scxml/state") {
            if let Some(state) = states.get_nodes_as_vec().first() {
                if let Some(name) = state.get_property("name") {
                    return name;
                }
            }
        }
    }
    "".to_string()
}

[cathaysia]

let Ok(xxx) = expr else {
    return Default::default()
};

maybe better?

[ZiCog]

@hurricane-socrates

I assume there's a way to avoid the redundant return.

On seeing that code the thing that shouts out as needing fixing is the horrendous mess of deeply nested if lets. As soon as I see code drifting right like that alarm bells go off in my head. I was amazed your concern was only about the return.

I'd go for something simple and regular like the 'let chains' suggested above.

[drmason13]

fn get_state(doc: &Document) -> Option<String> {
    Context::new(doc).ok()?
        .evaluate("/scxml/state").ok()?
        .get_nodes_as_vec().first()?
        .get_property("name")
}

fn get_default_state(doc: &Document) -> String {
    get_state(doc).unwrap_or_else(String::new)
}

This should also work. I've fudged a PoC playground without knowing what the functions involved really do.

[KSwanson]

Building off of @drmason13 's Playground, I wrote it in a functional way, if you prefer: Rust Playground

The fact that Vec::first returns a reference caused me to put the last and_then inside the second to last one...maybe if you futz around with it you can get it to Not Need That