Reducing the friction in the API for converting from Option to Result

Not long ago, I decided to refactor some code that I had written ala just make it work™ while quickly prototyping that looked something like this:

fn my_result_function() -> Result<MyStringWrapper, MyError> {
  let my_string: String = fn_returns_option_of_string()
    .unwrap_or_else(|| other_fn_returns_option_of_string()
      .unwrap_or_else(|| Some final fallback)
    )

  MyStringWrapper::parse(my_string) <- This returns a `Result<MyStringWrapper, MyError>`. Will soon refactor it to use `TryFrom` instead now that it's in the prelude.
}

After revisiting this code and giving it a more thoroughly thought, I decided to leverage the API for the Result rather than the API from the Option type, but I faced a lot of friction when doing it so, and the final code looks something like this:

fn my_result_function() -> Result<MyStringWrapper, MyError> {
  let my_string: String = fn_returns_option_of_string()
    .ok_or(MyError::SomeErrorVariant)
    .and_then(|the_string| {
       MyStringWrapper::parse(the_string)
    })
    .or_else(|| other_fn_returns_option_of_string()
      .ok_or(MyError::SomeErrorVariant)
      .and_then(|the_other_string| {
        MyStringWrapper::parse(the_other_string)
      })
      .or_else(|| MyStringWrapper::parse(Some final fallback)
    )
}

In other words, it went from cascading Option::unwrap_or_else to cascading Option::ok_or(...).and_then(...).or_else(...).

This has some nuances that I didn't like, like the completely unnecessary instance of MyError within the ok_or method call, just for converting the Option into a Result, which made me wonder if there's a better way to handle such cases or an incoming API for a more seamless conversion API for Option -> Result, and thus the reason for creating this thread to ask the community for their knowledge and expertise on this topic.

Why not chain .or_else() instead of .unwrap_or_else()? You would still need a final .unwrap_or_else(), but it would eliminate the nesting.

3 Likes

The sequence of ok_or-and_then-or_else is odd to me, because it means that the error you have with ok_or is never actually used because you ignore it in or_else, at which point I don't see why you flipped to using Result.

Can you maybe make a playground example of either version that compiles? (It doesn't need to do anything exciting, just compile -- for example or_else closures take an argument.) Then we can tweak that to show alternative approaches.

One thing I'd suggest is to use more ?. For example, you could make a little helper like

fn foo() -> Result<MyStringWrapper, MyError> {
    let the_string = fn_returns_option_of_string().ok_or(MyError::SomeErrorVariant)?;
    let y = MyStringWrapper::parse(the_string)?;
    Ok(y)
}

(This will eventually be easier inline with try_blocks - The Rust Unstable Book )

3 Likes

Yup. That's exactly what I meant when I mentioned the unnecessary instance of MyError.

I'm not using the question mark operator because I don't want to return early, I want to try all the possible fallbacks before I give up and try the "infallible" one.

I have created a dummy example in the playground. Just please ignore the over-simplification I had to use in it to have a somewhat similar code :sweat_smile: .

Well, try blocks will eventually help with that, but for now you can use the "IIFE operator" (||{ ... })() to scope where the ? goes.

With some macros, it could look like this, which I think expresses the intent fairly well:

fn my_result_function_option_based(random_number: usize) -> Result<MyStringWrapper, MyError> {
    let array = [random_number];
    let my_string: String = 
        coalesce_opt![
            fake_try_block! { let num = maybe_one(&array)?; Some(num.to_string()) },
            fake_try_block! { let num = maybe_two(&array)?; Some(num.to_string()) },
            fake_try_block! { let num = maybe_three(&array)?; Some(num.to_string()) },
            4.to_string(),
        ];
    MyStringWrapper::parse(my_string)
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=be9a0e3dbcfd661a3d89b3b27d2e8ba2

The macros:

macro_rules! coalesce_opt {
    ($a:expr $(,)?) => ( $a );
    ($a:expr , $($c:expr),+ $(,)?) => (
        if let Some(v) = $a {
            v
        } else {
            coalesce_opt!( $($c),+ )
        }
    );
}

macro_rules! fake_try_block {
    ($($a:tt)*) => ( (||{ $($a)* })() )
}
2 Likes

I don't see any need to refactor my_result_function_option_based into my_result_function_result_based at all... unless perhaps you're planning on changing the other methods from returning Options to returning Results, and were attempting to make that refactoring easier on yourself ahead of time.

Anyway, as others said, a good application of or_else is probably what you're looking for. Here's my refactoring of the example; I made my_result_function_result_based use functions that return Results to show you just need to change or_else(|| ...) into or_else(|_| ...). (If you're not changing the functions to return Results, just don't do the refactor.)

The only real "barrier" to this refactoring was storing the temporary &[random_number] at the top.

1 Like

Maybe some clarifications are needed to further advance in the discussion:

  • I'm consuming a third party API (a crate) that returns Option<&str>
  • The reason why I refactored it to use results was because of the use of the unwrap_or_else, which could return a &str that MyStringWrapper could not parse due to some domain constraints.
  • I know the playground example doesn't reflect all of these details. It's just a quick over-simplification to exemplify the type of friction I experienced when dealing with the existing API for converting from Option<T> into Result<T>.

That being said, the my_result_function_option_based just doesn't cut it because it's not sufficient to determine if one of the options is Some, it also needs to be parsable by MyStringWrapper before trying the next fallback in the chain.

As for my thoughts on what a good API could look like, first we need to call out the current issues:

  • Converting from Option<T> into Result<T> is currently done through the ok_or (eager) and ok_or_else (lazy) methods from the Option type
  • When the optional value held need to be passed to a Result itself, then we require to first convert the Option into a Result, and then use the monadic transformation and then, rendering the Error passed to ok_or useless

By simply following the naming convention of the APIs mentioned above, maybe a ok_and_then method in the Option type could alleviate the the friction by receiving a FnOnce(T) -> Result<U, E> to do the conversion. I'm unsure about the name for the lazy version of it, though.

Could a method like this be reasonable and idiomatic in Rust?

Oh, I see now... Sorry, I didn't catch that you were adding new logic in the second function.

ok_and_then sounds like .map(to_a_result).unwrap_or(Err(e)), where you still have to provide a "fallback error" for the None case. And for your use case, you'd still end up with a Result with an error you want to discard (except in the final, catch-all case).

However, you can end up in a similar place without the temporary error by using

// Option<T> -> Option<Option<U>> -> Option<U>
opt.map(|num| result_returning_fn(num).ok()).flatten()

Playground. The two functions do the same thing.

1 Like

Unless I'm missing something, there's no need for the unwrap_or in the result-based refactoring, because I'm already returning Result<MyStringWrapper, MyError>. The whole idea for the ok_and_then method is to make the monadic transformation from Option to Result in a single step.

Iirc, that was actually one of the different solutions I tried when refactoring the code. Since I'm using the Result type to add meaning to the unsuccessful cases and propagating it back to the edge of the API, I discarded it. I guess that you are still suggesting it because you still think that I'm trying to "unwrap" the Option, which is no longer the case. It was initially only because I wanted to pass a String to MyStringWrapper::parse. Correct me if I'm mistakenly reading the intention in your code suggestion.

Ah, I see! I think that last playground is definitely more succint :smiley: . Will give it a try!

Thanks, quinedot!

If the only inputs are an Option<T> (Self) and a F: FnOnce(T) -> Result<U, E>, please complete this code:

trait OptExt<T> {
    fn ok_and_then<U, E, F>(self, f: F) -> Result<U, E>
    where
        F: FnOnce(T) -> Result<U, E>;
}

impl<T> OptExt<T> for Option<T> {
    fn ok_and_then<U, E, F>(self, f: F) -> Result<U, E>
    where
        F: FnOnce(T) -> Result<U, E>
    {
        match self {
            Some(t) => f(t),
            None => {
                todo!()
                /* FILL THIS IN */
            }
        }
    }
}

It could work with E: Default I guess...

I get your point. Maybe some inspiration from the map_or method in the Option type would work:

trait OptExt<T> {
    fn ok_and_then<U, D, E, F>(self, default: D, f: F) -> Result<U, E>
    where
        D: FnOnce() -> Result<U, E>,
        F: FnOnce(T) -> Result<U, E>;
}

impl<T> OptExt<T> for Option<T> {
    fn ok_and_then<U, D, E, F>(self, default: D, f: F) -> Result<U, E>
    where
        D: FnOnce() -> Result<U, E>,
        F: FnOnce(T) -> Result<U, E>
    {
        match self {
            Some(t) => f(t),
            None => {
                default
            }
        }
    }
}

You could. That already exists as map_or_else though (except map_or_else doesn't require the returned type to be a Result specifically).

Yeah. But for all intents and purposes, the conversion from Result to Option is way more frictionless than the other way around, since converting Err to None can be done by simply ignoring the result's error (Err(_)) as in your proposed solution.

I'm ok with it, even if it requires the additional map and the unwrap_or_else to do the final conversion to Result.

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.