Style: Avoid ending with Ok(()) when possible?

I hope this isn't too trivial. Maybe this has been discussed before but I can't find it.

Which style is preferred?

Without Ok:

fn func() -> Result<()> {
    foo()?;
    bar()
}

With Ok:

fn func() -> Result<()> {
    foo()?;
    bar()?;
    Ok(())
}

The decision can be a little more complex when there are branches:

Without Ok:

fn write_thing() -> io::Result<()> {
    if (cond) {
        write!("thing")
    } else {
        write!("other thing")
    }
}

In other words, if the last expression happens to have the Result type to be returned, do you just return it? Or do you add ? and then Ok(())? Oftentimes, the latter approach feels more consistent and expressive. But on the other hand it costs an extra line of code (I know, big deal) and can seem like unnecessary noise.

1 Like

I think it depends. The rule I use is roughly the following: If removing the Ok(()) lets me remove all question marks, I remove the Ok(()).

So I would write func like this:

fn func() -> Result<()> {
    foo()?;
    bar()?;

    Ok(())
}

but write_thing like this:

fn write_thing() -> io::Result<()> {
    if (cond) {
        write!("thing")
    } else {
        write!("other thing")
    }
}
7 Likes

One issue I've run into is the error type. I run into problems with

fn foo() -> Result<(), Error> {
    do_something()
}

which gives a mismatched type on my custom Error. I'm forced to do

fn foo() -> Result<(), Error> {
    do_something()?;
    Ok(())
}

Annoying, but not enough to make me spend the time to find a better solution.

If you have any ideas, I've been using Failure since it was first released. Replacing it with something newer is probably a good idea but a lot of work.

1 Like

You could do_something().map_err(Error::from) to mimic the ? conversion.

6 Likes

I'd like to focus on the style question when it is possible skip Ok(()).

If you need an error conversion, I would use a question mark.

That works. It's more characters, but I think the intent is clearer.

For this pattern I'd suggest this way, because it lets you add more with a nicer diff.

Compare:

     foo()?;
-    bar()
+    bar()?;
+    qux()
 }

vs just

     bar()?;
+    qux()?;
     Ok(())

(Same reason as using trailing commas in enums or match arms even if they're not strictly needed.)

17 Likes

A variant of that is Ok(do_something()?).
Whether or not it's a good idea is a matter of preference.

As for the question: for me it's really contextual. For regular methods I think ending with Ok(()) is fine. But sometimes I have a method that takes a closure, and for brevity I let the Result of the inner code just flow out through so that the closure body can stay a one liner.

Rust doesn't have a good answer to this. There were proposals for ok-wrapping that would eliminate the syntax noise, but there are disagreements about the details of such feature.

1 Like

You can always Ok-wrap yourself, the only issue is that rustfmt adds rightwards drift when you do that, but then that's more of an issue with the formatter itself:

fn func () -> Result<()>
{Ok({
    foo()?;
    bar()?;
})}

/// or, given `macro_rules! ok {($($__:tt)*) => (Ok({$($__)*}))}`:
fn func () -> Result<()>
{ok!{
    foo()?;
    bar()?;
}}

one proposal that could end up happening is to use = expr instead of { ... } for a function body:

fn func () -> Result<()> = Ok({
    foo()?;
    bar()?;
})
// or
fn func () -> Result<()> = ok!{
    foo()?;
    bar()?;
}

Honorable mention: #[::fehler::throws]

#[throws(...)]
fn func ()
{
    foo()?;
    bar()?;
}
3 Likes

Where do I get to vote against that? Or anything else that makes a function definition even more complex?

We already have to pile enough stuff on to a line to define a function. What with all the life time ticks, generic types, type definitions, whereclauses etc, etc. often it's not immediately obvious that what you are looking it a function definition at all !

1 Like

I agree with @ZiCog that the value added by auto wrapping (be it Ok() or some other wrapper type) is not large enough to warrant making code less explicit and contain more "magic". Still not sure how I feel about Ok() wrapping in try blocks.

I personally don't mind the explicit Ok(()). It is not very elegant, but it is very clear. If the Result type of my last expression happens to match the return type of my function exactly, I tend to leave out the ? and the Ok(()).

2 Likes

Moderation note: Please don't turn this thread into another debate about the Ok-wrapping proposal. I see that it's related to the topic, but it's an entirely separate can of worms from the OP's question. If someone really feels inclined to open that can of worms again, then please 1) search the forum to review prior discussion and 2) open a new thread building on top of past discussion. I'll note that, at least as far as I'm aware, there hasn't been any recent movement on Ok-wrapping from the lang team. (So you might consider that now is not the time to raise the alarm if you feel strongly.)

11 Likes

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.