blog icon indicating copy to clipboard operation
blog copied to clipboard

Suggestion for making unwrap/rewrap more explicit in rust error handling guide

Open timmc opened this issue 6 years ago • 2 comments

I liked your guide to error-handling in Rust. One thing that stumped me for a bit, though, was the file_path_ext_explicit version of option-ex-string-find, which I'll paste here for convenience:

fn file_path_ext_explicit(file_path: &str) -> Option<&str> {
    match file_name(file_path) {
        None => None,
        Some(name) => match extension(name) {
            None => None,
            Some(ext) => Some(ext),
        }
    }
}

fn file_name(file_path: &str) -> Option<&str> {
  // implementation elided
  unimplemented!()
}

I understand that you were trying to illustrate the full case analysis without any cleanup, but it really bothered me that the inner match block was a complete no-op, and could have been replaced by Some(name) => extension(name). I wonder if comments might help here, perhaps something like:

    match file_name(file_path) {
        None => None,
        Some(name) => match extension(name) { // match to handle Option result
            None => None,
            Some(ext) => Some(ext) // Re-wrap ext to return an Option
        }
    }

...perhaps along with some note along the lines of "yes you could get rid of that match, but it's shown for pedagogical purposes here".

It's not a huge thing, but I did spend a bit of time checking if I'd missed something, so anything to indicate "no, it's not your imagination that this is weird" might be nice. :-)

timmc avatar Jul 09 '18 23:07 timmc

This is interesting. I think I'm with you, although looking at it now, I wonder if we could just elide the rewrapping altogether, since I agree that seems weird.

BurntSushi avatar Jul 10 '18 11:07 BurntSushi

Looking at it more, that might be the best option after all.

timmc avatar Jul 10 '18 13:07 timmc