About unwrap(), non-std Result and closures

It looks like unwrap() is very frowned upon in the Rust community. As a Rust newbie my problem is that sometimes I have a hard time trying to understand how to avoid it and write code in a more idiomatic way.

Just an example from a test code I'm working on:

pub struct RcdDir {
    curr: PathBuf,
    ...
}

impl fmt::Display for RcdDir { 
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        writeln!(f, "{}", self.curr.canonicalize().unwrap().to_str().unwrap())?;
        ...
    }
}

I can see that using the two unwraps() is ugly but what's the idiomatic way to write this silly code?

Should I use the family unwrap_or() / unwrap_or_else() or something entirely different?

On the same topic another related piece of code that made me question the usage of unwrap() was Vec::sort_by(), how can I avoid using unwrap() & company in the closure passed down to sort_by()? Should I just unwrap everything before the closure?

Typically, you would avoid the unwrap by doing these kinds of conversions in code that can properly handle/propagate the errors. In your example, you're storing a PathButh, but canonicalizing it before printing... but why? Why wouldn't you canonicalize it when you build RcdDir? Or when you modify PathBuf? Do the canonicalization earlier, in code that can report the error, and then you don't have to unwrap in the display impl.

Now that isn't universally good advice, because it really depends on what you're unwrapping and why. But for the most part, that's the idea: do things that can fail as early as possible. Don't do canonicalization in a display function. Separate your concerns.

EDIT:

This is a good case for unwrap_or/unwrap_or_else, because you probably want to provide a value to replace the failed one when you're sorting. If you don't have a good value, then you have a problem similar to the above, where you either are trying to sort unsortable data (which you should have handled prior) or you think it's ok for your program to panic when you sort (which is a reasonable option, especially if you're still early in the project lifecycle -- but something you'll likely want to fix eventually.)

3 Likes

I see your point and I agree with you, probably I should have thought this code a bit better (still learning).

On the other hand I still have some doubts about this. Let me explain my concerns with a new piece of code taken from the same program:

pub struct RcdEntry(DirEntry);

pub struct RcdDir {
    curr: PathBuf,
    entries: Vec<RcdEntry>,
}

impl Ord for RcdEntry {
    fn cmp(&self, other: &Self) -> Ordering {
        let meta_a = self.0.file_type().unwrap();
        let meta_b = other.0.file_type().unwrap();
        

        if meta_a.is_dir() && meta_b.is_file() {
            Ordering::Less
        } else if meta_a.is_file() && meta_b.is_dir() {
            Ordering::Greater
        } else {
            let name_a = self.0.file_name();
            let name_b = other.0.file_name();

            let str_a = name_a.to_str().unwrap().trim_start_matches('.');
            let str_b = name_b.to_str().unwrap().trim_start_matches('.');

            str_a.cmp(str_b)
        }
    }
}

The rationale here is that I want to have the directory / file entries (Vec<RcdEntry>) ordered not just by name but having the directories higher up in the list when doing entries.sort(). Since I have to have access to both the file name and file type, the only way I can have both information without having to add a new vector (one for the type another for the name) is by saving the RcdEntry / DirEntry of each entry and use .file_type() and .file_name().

Now, what the RUST experienced programmers would do here?

Why not store the name in the RcdEntry? Then you don't need to ask the OS every time you compare two elements.

Well, because the information would be redundant because the name is already "embedded" into DirEntry that I have to carry anyway because I need to access the metadata of the files.

1 Like

The unwrap appearing here means you're choosing to panic if the file_name is not valid UTF-8. That's fine if that's what you want, but if you really want to avoid panicking here, you either have to make sure your filenames are UTF-8 before you allow sorting (e.g., when you build the RcdEntry), or you must find another way to sort that doesn't care about UTF-8. OsString already implements Ord, so you might be able to find a way to avoid the to_str call altogether. Path has strip_prefix, so you might be able to find a way to get what you're looking for through there.

2 Likes

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.