3
\$\begingroup\$

I'm learning Rust. Could you share some tips for improving this code so it's more idiomatic and functional? I would prefer to stick to the standary library, and avoid unnecessary sorting or memory allocations.

fn first_file_in_dir(dir_path: &Path, extension: &str) -> Result<PathBuf, io::Error> {
    let mut matching_path = None;
    for entry in fs::read_dir(dir_path)? {
        let entry = entry?;
        if !entry.file_type()?.is_file() {
            continue;
        }
        let path = entry.path();
        if !path.extension().is_some_and(|x| x == extension) {
            continue;
        }
        if let Some(ref mut matching_path) = matching_path {
            if path < *matching_path {
                *matching_path = path;
            }
        } else {
            matching_path = Some(path);
        }
    }
    if let Some(matching_path) = matching_path {
        Ok(matching_path)
    } else {
        Err(io::Error::new(io::ErrorKind::NotFound,
                           format!("No *.{} file in {}", extension, dir_path.to_string_lossy())))
    }
}

The function returns the first file with a given extension inside a directory. Because the order of items from fs::read_dir() is platform dependent, it iterates over all files and picks the top one by name. Any I/O errors are propagated to the caller.

Thanks!

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

The code already looks fairly good. Some possible improvements:

Use cargo fmt

... to format the code according to recommended style guidelines.

Use clippy

... to lint your code. I recommend the additional options -- -W clippy::pedantic -W clippy::nursery -W clippy::unwrap_used.

Use built-in functions

You use Option::is_some_and(). You may as well use Option::ok_or_else().

Subjective opinion

Matching Some(ref mut) is okay, but I prefer to match against &mut option instead.

Suggested:

fn first_file_in_dir(dir_path: &Path, extension: &str) -> Result<PathBuf, io::Error> {
    let mut matching_path = None;

    for entry in fs::read_dir(dir_path)? {
        let entry = entry?;

        if !entry.file_type()?.is_file() {
            continue;
        }

        let path = entry.path();

        if !path.extension().is_some_and(|x| x == extension) {
            continue;
        }

        if let Some(matching_path) = &mut matching_path {
            if path < *matching_path {
                *matching_path = path;
            }
        } else {
            matching_path = Some(path);
        }
    }

    matching_path.ok_or_else(|| {
        io::Error::new(
            io::ErrorKind::NotFound,
            format!("No *.{} file in {}", extension, dir_path.to_string_lossy()),
        )
    })
}

Alternative implementation

If you don't mind ignoring erroneous entries, you can even write the method as one expression:

pub fn first_file_in_dir(dir_path: &Path, extension: &str) -> Result<PathBuf, io::Error> {
    fs::read_dir(dir_path)?
        .filter_map(|result| {
            result.ok().and_then(|entry| {
                entry.file_type().ok().and_then(|file_type| {
                    if file_type.is_file()
                        && entry.path().extension().is_some_and(|x| x == extension)
                    {
                        Some(entry.path())
                    } else {
                        None
                    }
                })
            })
        })
        .reduce(|lhs, rhs| if lhs < rhs { lhs } else { rhs })
        .ok_or_else(|| {
            io::Error::new(
                io::ErrorKind::NotFound,
                format!("No *.{} file in {}", extension, dir_path.to_string_lossy()),
            )
        })
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks! I'll enable the extra clippy lints, they are sure to come in handy. The alternative implementation is interesting. I managed to simplify it a bit. It would be real nice if Rust had a way to keep code so terse, while still propagating the errors. \$\endgroup\$ Feb 27 at 20:18
  • \$\begingroup\$ That's neat. I didn't know about Iterator::min(). \$\endgroup\$ Feb 28 at 9:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.