Search code examples
rustunwrap

Replacing unwrap() with ? made code more complicated instead of simplifying it


This code deletes PNG files in a folder and prints them. I wanted to simplify it a bit by replacing all the unwrap()'s with ?.

use std::fs;
use std::path::Path;

fn main() -> Result<(), std::io::Error> {
    let path = Path::new("/home/alex/Desktop");
    for entry in fs::read_dir(path)? {
        let entry = entry?;
        let path = entry.path();
        if path.is_file() && path.extension().unwrap() == "png" {
            fs::remove_file(&path)?;
            println!("{}", path.file_name().unwrap().to_str().unwrap());
        }
    }
    Ok(())
}

I found out I can't replace the unwrap()'s that are handling Option instead of Result. In this case, extension(), file_name(), and to_str().

I changed the code to solve that problem. However, the code just became more complicated:

use std::fs;
use std::path::Path;

fn main() -> Result<(), std::io::Error> {
let path = Path::new("/home/alex/Desktop");
for entry in fs::read_dir(path)? {
    let entry = entry?;
    let path = entry.path();
    if path.is_file() {
        let ext = path.extension().ok_or(std::io::Error::new(std::io::ErrorKind::Other, "Invalid file extension"))?;
        if ext == "png" {
            fs::remove_file(&path)?;
            println!("{}", path.file_name().ok_or(std::io::Error::new(std::io::ErrorKind::Other, "Invalid file name"))?.to_str().ok_or(std::io::Error::new(std::io::ErrorKind::Other, "Invalid file name"))?);
        }
    }
}
Ok(())}

How to replace the unwrap()'s that are handling Option without making the code more complicated (especially, without so much nesting)? Or at least not as complicated as the one I shared?


Solution

  • In many of the cases where you were calling unwrap it wasn't actually an error, you just want to do something different (or not at all).

    If you're only interested in the case where there is a png extension, check if that's what you got. It doesn't matter if it's None or .jpg.

    Printing a filename can fail because filenames can have non unicode characters and Rust strings can't. In this case given it's presumably meant to be human readable, printing the to_string_lossy() output (replacing non-unicode characters) or printing it using debug mode (escaping non-unicode characters) is probably fine.

    use std::fs;
    use std::path::Path;
    
    fn main() -> Result<(), std::io::Error> {
        let path = Path::new("/home/alex/Desktop");
        for entry in fs::read_dir(path)? {
            let entry = entry?;
            let path = entry.path();
            if path.is_file() {
                if let Some(ext) = path.extension() {
                    if ext == "png" {
                        fs::remove_file(&path)?;
                        // I would keep expect here as extension would return None if there was no filename
                        // to_string_lossy returns a printable string - possibly replacing non-unicode characters
                        println!("{}", path.file_name().expect("File has no name").to_string_lossy());
                    }
                }
            }
        }
        Ok(())
    }