Provide a common API across `Option` and the `Ok` and `Err` variants of `Result`.

[brendanzab]

Rendered

[bstrie]

I'm sad that this appears to make error handling uniformly more verbose.

The inconsistency will also make it more challenging to transition to an API with a single trait that abstracts over Option-style things once higher-kinded types are implemented post-1.0.

What would this actually allow us to do, in practical terms? Feel free to invent syntax as needed.

[huonw]

This appears to have HTML mangling?

[brendanzab]

Yeah, I had to use those because the pipes were interacting with the markdown table syntax.

[thehydroimpulse]

@bstrie

What would this actually allow us to do, in practical terms? Feel free to invent syntax as needed.

Only a single trait will be required to implement the common methods. Separate implementations will be used for Option or Result specific methods.

// Methods in common with Result and Option.
trait Optional<M[T]> {
    pub fn unwrap_or(self, def: T) -> T { ... }
    pub fn unwrap_or_else(self, f: || -> T) -> T { ... }
}

impl<M[T]> Optional<M[T]> for Option<T> {
    // ...
}

// ...

The M[T] denotes a higher-kinded type (sometimes, you'll use M[_]). I'm not sure if that syntax will fly with everyone, though. I'm almost done an RFC for it, so we'll see.

So in short, we're allowed to abstract over more complex types.

[bstrie]

I was mostly curious as to how this would make the usage of Option nicer, rather than its implementation. I really couldn't care less about the implementation, as long as it's as nice to use as we can make it.

[thehydroimpulse]

Gotcha.

[chris-morgan]

If the rest is accepted, +1 on this.

[chris-morgan]

Once you’ve got that, you’re back to the current situation with Result.ok() and Result.err(), and so the actual effect of this is simply “remove all the methods on Result except for ok and err.

[brendanzab]

Indeed, I am not really a fan of this. I think implementing the most common case directly on Result is the right way to go.

[wycats]

Agreed. Treating Result as a kind of Option with optional error information is nice.

[Valloric]

I really couldn't care less about the implementation, as long as it's as nice to use as we can make it.

This. The user doesn't care how nicely refactored the compiler internals are.

[brendanzab]

I really couldn't care less about the implementation, as long as it's as nice to use as we can make it.

That is the point. To simplify things by providing a set of standard methods. You only have to remember one set of methods rather than two, inconsistent APIs.

[bstrie]

I apologize if I seem grumpy. My complaints stem from the fact that I've never believed that our Option handling is as nice as it needs to be, given how fundamental to the language that it is. Other languages with Options all seem to have mechanisms to ease usage (Haskell, Swift, C#). I fear that people familiar with these languages will judge us unfavorably if idiomatic Option handling is overly clunky.

[schmee]

Why not introduce some sugar for Option? I think Swift does this really nicely (at least it seems nice, I haven't written any Swift code), with T? as a shorthand for Option<T> and lots of sugar for unwrapping and chaining.

[sinistersnare]

As I understand it, is the reason that we do not give special sugar for Option type (or any of the Result types) is because the language designers do not want to 'bless' types? Maybe providing a couple general overloadable unary operators might be the answer to things like T?

[thehydroimpulse]

I really couldn't care less about the implementation, as long as it's as nice to use as we can make it.

I agree. In this situation, it would provide a much simpler implementation and a much nicer interface for users. That's with the introduction of monads and potentially re-introducing the do keyword (or do macro) similar to Haskell's. That's the benefit of HKTs in this scenario.

Here's an example using a fictitious do keyword:

do! {
    action1();
    action2();
    action3();
}

fn action1() -> Option<int> {
    return Some(5);
}

fn action2() -> Option<int> {
    return Some(55);
}

fn action3() -> Option<String> {
    return "foobar".to_string();
}

Moreover, one could bind the value of T from each operation:

do! {
    let a <- action1();
    let b <- action2();
    action3(a, b);
}

fn action3(a: int, b: int) -> Option<String> {
    return "".to_string();
}

The reason for having let a -> instead of let a = is because we aren't binding to the return value of action1() or action2() but the value within the Option.

This would ideally all be done through macros, so as to not give special treatment to the few types.

I find the HKT and monad abstraction to be much more appropriate than simple sugar like T?. This is also just a brief example on how HKTs would allow for a better user experience than just changing some method names.

(You would still need to handle a fail case, otherwise it might fail for you)

[bstrie]

Is there a reason that we can't implement the do! macro today, in the same vein as the try! macro? Would the API unification presented in this RFC mean that we could use a single macro of this type for both Options and Results?

[thehydroimpulse]

do! is simply sugar on top of a monad's bind operator (>>= in Haskell). try! is significantly simpler (just a simple pattern match) whereas a monad requires us to be able to abstract over a type constructor.

I haven't put much thought about how this would all affect Result, however. The do! macro would simply return an Option<T>.

So one could do:

fn something_important() -> Option<String> {
    return do! {
        let a <- action1();
        let b <- action2();
        action3(a, b);
    };
}

Versus:

fn something_important() -> Option<String> {
    match (action1(), action2()) {
        (Some(a), Some(b)) => {
            match action3(a, b) {
                Some(s) => Some(s),
                None => return None
            }
        },
        _ => return None
    }
}

So it would result in composition similar to Result with try. However, try! is super simple.

[aturon]

I think it's worth separating out two aspects of this RFC:

1. The Option and Result types could (and should?) implement some common interface. This makes it easier to transition code from one to the other, and makes it easier to remember the API. In today's API, there is a rather uneven selection of convenience methods available on the different types, especially when it comes to the Ok versus the Err variants of Result.
2. The common interface should be "cleaned up" by reducing the number of convenience methods and instead reusing methods like as_ref.

It's hard to argue against the first point: it seems pretty clear that the overall API will be improved if we make the two types support a common interface. Is there any disagreement about that?

@bjz, perhaps it's worth outlining an additional alternative for your design that addresses point 1 above but not point 2, i.e. keeps more of the convenience methods currently available for Option and makes them available for Result as well?

Here are my thoughts on point 2:

• I think dropping get_ref, get_mut, and take_unwrap are all fine, as the expanded form of each is only slightly longer and especially for get_ is more clear.
• For the iter methods, I would rather leave in the existing convenience methods, because they are part of a widespread convention in the libraries for producing iterators for a type. (Whether this convention is the right one is a separate issue).
• For the as_slice and as_mut_slice methods, the expanded form is pretty terrible, so I think these are conveniences worth having.

[brendanzab]

@aturon Good points. Re. as_slice and as_mut_slice, are those used all that often? They seemed a tad extraneous to me - but I could be wrong.

[aturon]

@bjz I don't have any data here, but my feeling is: they are simple and easy to understand, especially given the as_ref and as_mut methods -- they're part of a family, and it seems reasonable to include the whole set.

[brendanzab]

Ok, I restored the old iterator and slice conversion methods and mirrored them for Result. Hopefully this reduces the scope somewhat.

[pczarn]

I prefer the ForOk/ForErr symmetry suggested as an alternative. However, I would incorporate these adapters straight into the type Result<T, E, S = Ok> in order to improve expressiveness. The result can be in one of two states that determine the API's "bias".

There's a way to change the type without discarding the other value.

Type	Implements method(s)	With operations
Result<T, E, Ok>	ok()	(no operation)
Result<T, E, Ok>	err()	casts to a type where S = Err
Result<T, E, Ok>	to_option()	Converts to Option<T>, discarding the error.
Result<T, E, Err>	ok()	casts to a type where S = Ok
Result<T, E, Err>	err()	(no operation)
Result<T, E, Err>	to_option()	Converts to Option<E>, discarding the value.
Result<T, E, _>	all in common with Option	as described in the RFC

Additionally, old methods ok and err can be renamed to into_ok and into_err for convenience:

Old API	Proposed API
.ok()	.into_ok() or .ok().to_option()*
.err()	into_err() or .err().to_option()*
.map_err(...)	.err().map(...)
.or_else(...)	.err().or(...)

* Since to_option is supposed to consume the result, shouldn't it be called into_option?

[brendanzab]

So are you saying that you would have two phantom types, enum Ok {} and enum Err {}?

[pczarn]

Yes, either those enums or struct Ok; and struct Err;.

[brendanzab]

They would have to be phantom types if they were going to reuse the same identifiers as the Result variants. Otherwise the constructors would conflict. I'm a little concerned about the 'complexity factor' of this proposal though.

[cmr]

I'm in favor of this. Seems like a clean refactor to a consistent API. Consistency is king.

[brendanzab]

@cmr Thoughts on the phantom type idea?

[cmr]

The complexity does not pay for itself, I think.

[aturon]

@pczarn Can you elaborate on what you see as the benefits of using phantom types here? It looks like Result<T, E, Ok> is @bjz's Result<T, E> and Result<T, E, Err> is @bjz's ForErr<T, E>.

Is your proposal mainly about the aesthetics of the types (just one type with a parameter versus two types), or does it make client code nicer/easier in some way?

[bstrie]

I'm fine with this. Consistency is good. And if people decide that they need even more convenience methods in the future, we just need to be sure that they can be implemented sanely for both types.

[aturon]

I am in favor of this RFC, modulo one concern: I don't understand why the expect method is singled out and put into its own trait. My understanding was that eventually, with HKT, we could have an "OptionLike" trait encompassing the common interface. Why treat expect differently?

Also, minor note: the Result type already contains an unwrap method, but only when the error type is Show. The RFC lists it as an added method, and does not require Show. Do you plan to keep the current behavior, or change it?

[brendanzab]

The expect method requires std::Any, which is why it is currently in an extension trait in the std::option module. We could always make people do unwrap_or_else(|| fail!("...")), instead ofexpect("...")`, but that is a great deal more typing.

[brendanzab]

Oh wait, Any is defined in core! 😅

[brendanzab]

@aturon: re. unwrap, it seems like it would be hard to keep the current behavior of Result::unwrap, which prints out the error type on failure whilst keeping the type bounds consistent with Option :/

[brendanzab]

Oh, it seems that the std::option::Expect trait was removed in rust-lang/rust#14610

[aturon]

@bjz I think it's fine to leave unwrap as-is: it will have the same signature across Result and Option, but will only be available on Result when the error type is Show.

I wonder whether for_err could just be err, since the old err method is being dropped? That would cut down some of the verbosity on error-related methods.

Also, to_option should probably be into_option.

[aturon]

@bjz Sorry, just noticed you suggested err in your alternatives section. Do you have an argument in favor of for_err instead?

[alexcrichton]

We talked about this in person yesterday, but I wanted to write this down for posterity:

This proposal means that

foo.map_err(|e| /* ... */)

would become

foo.for_err().map(|e| /* ... */).to_result()

which I sadly find quite wordy :(

[alexcrichton]

Or rather, you considered it a mild bug that ForErr::map returned Result instead of ForErr, and if it were to be fixed it would imply the above code.

[brendanzab]

Thanks for writing these things down - I will update the PR accordingly today.

[aturon]

@alexcrichton FWIW, I think it's OK for the ForErr methods to yield Results, since (presumably?) most of the time you're doing a single error-biased operation. If you also make the biasing err rather than for_err, you get

foo.err().map(|e| /* ... */)

rather than today's

foo.map_err(|e| /* ... */)

which is not too bad.

@bjz I realized this morning that the semantics of methods like and and or on ForErr are not completely obvious. Can you spell them out?

[brendanzab]

@aturon re. the semantics of and and or:

impl<T, E> Result<T, E> {
    pub fn and<U>(self, other: Result<U, E>) -> Result<U, E> {
        match self {
            Ok(_) => other,
            Err(e) => Err(e),
        }
    }

    pub fn or(self, other: Result<T, E>) -> Result<T, E> {
        match self {
            Ok(x) => Ok(x),
            Err(_) => other,
        }
    }
}

impl<T, E> ForErr<T, E> {
    pub fn and<F>(self, other: Result<T, F>) -> Result<T, F> {
        match self {
            ForErr(Err(_)) => other,
            ForErr(Ok(x)) => Ok(x),
        }
    }

    pub fn or(self, other: Result<T, E>) -> Result<T, E> {
        match self {
            ForErr(Err(e)) => Err(e),
            ForErr(Ok(_)) => other,
        }
    }
}

[aturon]

@bjz Thanks for the elaboration.

I have mixed feelings about these methods. The definitions you give here are somewhat symmetric to those in Result, except that they combine a ForErr with a Result and produce a Result. The outcome is that the names and and or here aren't terribly intuitive (to me at least).

Would it make sense to have them take ForErr as other instead? Are there other alternatives? Or can you give a good intuition for the definitions you gave?

Perhaps related: if you did eventually want to write a trait encompassing the Option/Result/ForErr APIs, the ForErr methods that currently take or return any Result values would instead need to take/return ForErr (which would be Self in the trait). But that comes at an ergonomic cost for the map method...

[brendanzab]

Yeah, as we figured out the other day, for them to satisfy a truly common API the ForErr methods need to take and return ForErrs. This would result in more verbose error handling.

[aturon]

@bjz Right -- I wanted to get that in the public record :-)

But a broader point is, if we're not actually making ForErr match a "truly common" API, we should give careful thought to which forms of and/and_then/or/or_else are most intuitive and useful on ForErr. Can you give an argument for why the signatures/implementations you gave are the right ones?

[brendanzab]

No reason - I think it was a brain-fart on my part :)

[bluss]

The proposal seems to add net complexity rather than remove it. Result gets new methods for iteration and slicing, and the result module a whole new type ForErr that users will be confronted with when reading the documentation.

I think -- my perspective is simpler API, simpler usage -- this proposal should be abandoned, take a step back, look at smaller things that can be adjusted in Result and Option -- for example removing .take_unwrap().

[nielsle]

Slightly orthogonal: It would be nice to have a simple method for converting an Option to a Result<T,E>.

pub fn ok_or<E>(self, f: || -> E) -> Result<T,E> { ... }

That would allow

let x: u32 = try!(from_str("25").ok_or(|| MyConversionError))

[nielsle]

Perhaps E should be required to implement Show.

impl<T, E: Show> Expect<T> for Result<T, E> {

That would allow rust to print the error message when failing to unwrap, and hopefully that would help newbies dealing with Results in main().

See also a relevant discussion of a commit to "Guide: Standard input":
rust-lang/rust#15534 (comment)

[brendanzab]

Ok, @aturon and I are thinking that adding a ForErr adaptor struct is a big change, and I there are a number of issues with my proposed API for the type. That said, making the Result API consistent with Option is important, so @aturon will draw up a new RFC addressing that. map_err will remain in place for now. We can always tackle ForErr at a later date.

[aturon]

@bjz With the Option/Result stabilization that's about to land, I think we've covered the conservative version of the proposal here. Should probably close this RFC.

[pczarn]

I think the phantom type idea would easily work with HKT. That's the only benefit I see. (I'm surprised this RFC is still open)

[brendanzab]

@aturon Ok, I'll close this now then!