Idiomatically return Result<Option> if Option::None else use value

Assume I have a Vec<String> and I want to retrieve the item at index i. The item might not be present there, so I use Vec::get(), which gives me Option<String> (Note: the vector was preallocated with with_capacity).

My method returns Result<Option<String>, MyError> and I want to return Ok(None) if the value is not present in the array and work with the value otherwise.
Now I know I can use either match or

let items_opt = self.items.get(index);
if items_opt.is_none() {
    return Ok(None);
}
...

to return what I want. But is there a more idiomatic way. Would this be acceptable?

if let Some(item) = self.items.get(index) {
    ...
}
else { 
    Ok(None)
}
1 Like

I would probably write this:

let item = match self.items.get(index) {
    Some(item) => item,
    None => return Ok(None),
};
6 Likes

This is also a perfect use case for the probably-soon-to-be-stable let..else construct:

let Some(item) = self.items.get(index) else { return Ok(none) };
6 Likes

In this case the second one is preferable because if you need to do some work on item you'll still need to destructure Option<Item> which requires calling unwrap() at some point:

let item_opt = self.items.get(index);
if item_opt.is_none() {
    return Ok(None);
}
let item = item_opt.unwrap();
...

Although in more complex cases the first one is preferable:

fn to_stuff(animal: Animal) -> Result<Value> {
    if animal.is_plankton() {
        return Ok(None);
    }

    if animal.legs() > 4 {
        return compute_insect(animal);
    }

    let food = match animal.color {
        ...
    };

    if  animal.has_fangs() {
        return do_stuff_with_food(food);
    }

    if let Some(food) = food {
        Ok(also_do_stuff_with_food(food))
    } else {
        Ok(None)
    }

}

Also when the work that you do with item has deep indentation, prefer the first one

if item_opt.is_none() {
    return Ok(None)
}

{
    ...
    {
        ...
        {
            ...
        }
    }
}

How about:

let x: Result<Option<String>, _> = items.get(index).map(|x| {
    todo!()
    Ok(x.clone())
}).transpose()

The key insight here is that we can use transpose to swap Option and Result

1 Like

I'll certainly write poop :blush:

It's not very idiomatic I think but in some cases... in an internal code... :shushing_face:

If we work nightly and with some features, we could write our own error if it becomes inconvenient to work with Option and Result. We can work with some ErrorKinds or with the try_trait feature.

So it can look like:

#![feature(try_trait_v2)]
#![feature(never_type)]

enum MyResult<T, E> {
    Some(T),
    // any variant you want
    None,
    Err(E),
}

impl<T, E> std::ops::Try for MyResult<T, E> {
    type Output = T;
    type Residual = MyResult<!, E>;

    #[inline]
    fn from_output(c: T) -> Self {
        MyResult::Some(c)
    }

    #[inline]
    fn branch(self) -> std::ops::ControlFlow<Self::Residual, T> {
        match self {
            MyResult::Some(c) => std::ops::ControlFlow::Continue(c),
            MyResult::Err(e) => std::ops::ControlFlow::Break(MyResult::Err(e)),
            MyResult::None => std::ops::ControlFlow::Break(MyResult::None),
        }
    }
}

impl<T, E, F: From<E>> std::ops::FromResidual<MyResult<!, E>> for MyResult<T, F> {
    fn from_residual(x: MyResult<!, E>) -> Self {
        match x {
            MyResult::Some(e) => MyResult::Some(From::from(e)),
            MyResult::None => MyResult::None,
            MyResult::Err(e) => MyResult::Err(From::from(e)),
        }
    }
}

fn bar() -> MyResult<String, &'static str> {
    // MyResult::Some(String::from("haha!"))
    // MyResult::None
    MyResult::Err("This is an error")
}

fn foo() -> MyResult<String, &'static str> {
    let _ = bar()?;
    MyResult::Some(String::from("hello world"))
}

fn main() {
    match foo() {
        MyResult::Some(hello) => println!("{}", hello),
        MyResult::None => println!("nothing O_O"),
        MyResult::Err(e) => println!("{}", e),
    }
}

Any opinion? :smile:

I'll keep that in mind for future versions. Thanks!

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.