Don't do `type Result<T> = std::result::Result<T, MyError>;`

I'm normally not a guy for rants but recently I started to write small PRs for some Rust crates and this is something that bothers me very often.

tl;dr

Don't:

type Result<T> = std::resutl::Result<T, MyError>;

Do:

type Result<T, E = MyError> = std::result::Result<T, E>;

Long version

With crates like thiserror, snafu and so on it is common to use a custom error type. This is good practice and I support it. However, what usually comes along with a custom error type is a convenient type alias for Result. Often this looks like the following:

type Result<T> = std::resutl::Result<T, MyError>;

This works most of the time but when someone has to implement a trait with fallible methods or wants to use a different error type somewhere the usual -> Result<T, OtherError> results in a compiler error, e.g. here (playground):

struct MyError;
type Result<T> = std::result::Result<T, MyError>;

struct Foo;
struct ParseFooError;

impl FromStr for Foo {
    type Err = ParseFooError;
    
    fn from_str(s: &str) -> Result<Self, Self::Err> {
        Err(ParseFooError)            // ^^^^^^^^^ unexpected type argument
    }
}

Now we can either use a fully qualified name in this one case std::result::Result or create a new type alias ParseFooResult. Instead, I strongly recomment the more convenient to use alias:

type Result<T, E = MyError> = std::result::Result<T, E>;

This version sets MyError as the default parameter which still allows to override the parameter if required (like in the example, go try it out and replace the type alias in the playground).


That's it. If someone has a different opinion on this matter I'll be happy to read the reasons.

15 Likes

Would you mind elaborating a bit? I might be missing the point -- from my limited experience I'd tend to write

fn from_str(s: &str) -> std::result::Result<Self, Self::Err> { ...

instead of

fn from_str(s: &str) -> Result<Self, Self::Err> { ...

In other words, I'd use Result for common cases and for individual cases use std::result::Result. Using Result<Self, Self::Err> is less typing but I actually prefer to see the full std::result::Result when reading the code, especially in non-IDE environments like diffs where it's difficult to see which type alias is what.

I'm thinking it might make a difference to the caller if they need to do something with the Result type. If it that's the case, I'd like to learn more about it.

1 Like
  1. std::result::Result is super verbose to write each time.

  2. If you write a library with a Result alias, other people using your library may do use library::* and end up with a "wrong" Result by accident. It makes life easier for everyone, even if you personally take care to avoid it.

Might be just me I find both examples good examples against using the OP approach. :slight_smile:

  1. std::result::Result is super verbose to write each time.

It's verbose to the writer but the reader can parse it immediately without conscious effort. My experience is that there are too many Results all over the place and knowing I'm dealing with the standard one makes it easier to read and understand the code. Especially since I don't have to think about:

  1. If you write a library with a Result alias, other people using your library may do use library::* and end up with a "wrong" Result by accident. It makes life easier for everyone, even if you personally take care to avoid it.

This is my main problem with type aliases. They usually sound like a good idea but in the end when each module has its own Result, reading a PR I'll end up doing lots of scrolling and expanding. This takes my focus away from the real change.

That's why I asked if there's any other purpose other than not having to write std::result:: in a couple of places.

1 Like

The thing is that actually every type alias of Result refers to the same type(-constructor) std::result::Result so being required to write the fully qualified name to refer to the same type seems wrong to me.

That's exactly the point. With the default error parameter you know for the whole module which default error is used (there can only be one use ...::Result;) everywhere another error is required you have to write it explicit.

I prefer not to have such type alias at all. I always type the full Result<T, Error> everywhere, or name it differently (eg. use std::fmt::Result as FmtResult), for several reasons:

  • I feel like Result name is already taken by a high-profile in the prelude type. Seeing Result, it always means that one to me.
  • It's not really obvious what error it can return. It's another click for me in the docs when a library uses its own Result, because I want to see the variants of the error or something.
  • I've seen newbies utterly confused about „Why can Result have only one type parameter here?“
1 Like

One obvious and readable solution on part of the consumer of the library (which doesn't even need the support of the library author) is to write:

use foobar::errors::Result as FoobarResult;
use transmogrify::prelude::Result as XmogrifyResult;

etc.

1 Like

As a reader, I don't know that. I have to look at the code that defines the type whether it's an alias or something else. It becomes tedious if I have to do it for every type so in the end I get lazy and make (sometimes wrong) assumptions.

Another thing is documentation, if you document the alias and then use it by accident instead of the standard one, a newcomer to the project will be wondering why foo::bar::Result is used.

I like Result to represent the common result the module is returning. For generic Results I prefer to use the standard one. It's clearer and less surprising to the reader (when you have multiple readers with different range of skills/experience.)

To take it one step further, you can also do

type Result<T = (), E = Error> = std::result::Result<T, E>;

fn foo() -> Result {
    Ok(())
}

(mileage may vary, depending on how many functions you have returning Ok(())).