Let's focus on Result; I'll go back to Option at the end.
The purpose of Result is to signal a result which may succeed or fail with an error. As such, any use of it should fall into this category. Let's ignore the cases where a crate returns Result for impossible-to-fail operations.
By doing what you are doing (checking if result.is_ok() then extracting the value), you're effectively doing the same thing twice. The first time, you're inspecting the content of the Result, and the second, you're checking and extracting unsafely.
This could indeed have been done with a match or map, and both would have been more idiomatic than an if. Consider this case for a moment:
You have an object implementing the following trait:
use std::io::{Error, ErrorKind};
trait Worker {
fn hours_left(&self) -> Result<u8, Error>;
fn allocate_hours(&mut self, hours: u8) -> Result<u8, Error>;
}
We're going to assume hours_left() does exactly what it says on the tin. We'll also assume we have a mutable borrow of Worker. Let's implement allocate_hours().
In order to do so, we'll obviously need to check if our worker has extra hours left over to allocate. You could write it similar to yours:
fn allocate_hours(&mut self, hours: u8) {
let hours_left = self.hours_left();
if (hours_left.is_ok()) {
let remaining_hours = hours_left.unwrap();
if (remaining_hours < hours) {
return Err(Error::new(ErrorKind::NotFound, "Not enough hours left"));
}
// Do the actual operation and return
} else {
return hours_left;
}
}
However, this implementation is both clunky and inefficient. We can simplify this by avoiding unwrap and if statements altogether.
fn allocate_hours(&mut self, hours: u8) -> Result<u8, Error> {
self.hours_left()
.and_then(|hours_left| {
// We are certain that our worker is actually there to receive hours
// but we are not sure if he has enough hours. Check.
match hours_left {
x if x >= hours => Ok(x),
_ => Err(Error::new(ErrorKind::NotFound, "Not enough hours")),
}
})
.map(|hours_left| {
// At this point we are sure the worker has enough hours.
// Do the operations
})
}
We've killed multiple birds with one stone here. We've made our code more readable, easier to follow and we've removed a whole bunch of repeated operations. This is also beginning to look like Rust and less like PHP ;-)
Option is similar and supports the same operations. If you want to process the content of either Option or Result and branch accordingly, and you're using unwrap, there are so many pitfalls you'll inevitably fall into when you forget you unwrapped something.
There are genuine cases where your program should barf out. For those, consider expect(&str) as opposed to unwrap()
