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:
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.
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)
}
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 .
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.
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
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.
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.