4
\$\begingroup\$

I'm a C++ developer trying to learn Rust for fun and XP. I decided to implement some of the Unix tools for practice.

Below is a simplified implementation of the find command. The program takes 0 to 2 arguments: The first being the file to find, the second being the directory to start searching (I'm aware that the args aren't very robust).

I get the feeling there is a more succinct way to do what I'm trying to accomplish.

use std::env;                                                                                                                                                                                  
use std::io;                                                                                                                                                                                   
use std::collections::VecDeque;                                                                                                                                                                
use std::ffi::OsString;                                                                                                                                                                        
use std::path::PathBuf;                                                                                                                                                                        
use std::process::exit;                                                                                                                                                                        

fn find(query: &String, start: &OsString) -> io::Result<Vec<PathBuf>> {                                                                                                                        
    let start = PathBuf::from(start);                                                                                                                                                          
    let mut dirs = VecDeque::from(vec![start]);                                                                                                                                                
    let mut result = Vec::new();                                                                                                                                                               

    while let Some(dir) = dirs.pop_front() {
        for entry in dir.read_dir()? {
            let path = entry?.path();
            if path.is_dir() {
                dirs.push_back(path.clone());
            }
            if let Some(name) = path.file_name() {
                if query.is_empty() || query.as_str() == name {
                    result.push(path.clone());
                }
            }
        }
    }
    Ok(result)
}
fn main() {
    let query = match env::args().nth(1) {
        Some(query) => query,
        None => String::new(),
    };
    let start = match env::args().nth(2) {
        Some(start) => OsString::from(start),
        None => OsString::from("."),
    };

    match find(&query, &start) {
        Ok(paths) => {
            for path in paths {
                if let Some(p) = path.to_str() {
                    println!("{}", p);
                }
            }
        },
        Err(err) => {
            eprintln!("Error: {}", err);
            exit(1);
        },
    };
}
\$\endgroup\$
  • \$\begingroup\$ @Ludisposed yes. If I understand this correctly, it works because it is creating a copy of path (much like C++ would do implicitly). \$\endgroup\$ – zachwhaley Aug 3 '18 at 14:53
  • \$\begingroup\$ Thanks @Ludisposed Do I need to do anything to avoid it being closed? \$\endgroup\$ – zachwhaley Aug 3 '18 at 15:00
3
\$\begingroup\$

Your code can be simplified a little bit:

use std::collections::VecDeque;
use std::ffi::{OsStr, OsString};
use std::io;
use std::path::PathBuf;

fn find(query: &str, start: &OsStr) -> io::Result<Vec<PathBuf>> {
    let start = PathBuf::from(start);
    let mut dirs = VecDeque::from(vec![start]);
    let mut result = Vec::new();

    while let Some(dir) = dirs.pop_front() {
        for entry in dir.read_dir()? {
            let path = entry?.path();
            if path.is_dir() {
                dirs.push_back(path.clone());
            }
            if let Some(name) = path.file_name() {
                if query.is_empty() || query == name {
                    result.push(path.clone());
                }
            }
        }
    }
    Ok(result)
}

fn main() -> io::Result<()> {
    let mut args = std::env::args().skip(1);
    let query = args.next().unwrap_or(String::new());
    let start = args.next().map(OsString::from).unwrap_or(OsString::from("."));

    for path in find(&query, &start)? {
        if let Some(p) = path.to_str() {
            println!("{}", p);
        }
    }
    Ok(())
}

In your function find, you should use the borrowed version of String and OsString as parameters; or you can just move the value if you think that query or start will not be needed anymore in the main.

In you main, you can take advantage of the Iterator's and Option's methods to make a more concise and cleaner code:

  • You create an iterator that will iterate over all args but the first using skip;
  • The query is the next item: if there is no item, you give a default value with unwrap_or;
  • For the next item, first you apply a function to the inner with map, and then you provide a default value;
  • Your main can return a Result, so you do not need to make the error handling by hand.

Just a little thing: the idiomatic way to do such an algorithm in Rust is to implement an Iterator. In you code you do an unnecessary allocation for all your found files.

\$\endgroup\$
  • \$\begingroup\$ Thanks! Didn't know about main being able to return a Result o_O What about the calls to clone() on path. I felt weird adding those calls in, but it seemed necessary to get rid of some move errors I was seeing. \$\endgroup\$ – zachwhaley Aug 3 '18 at 15:34
  • \$\begingroup\$ Also, are you suggesting find() should return an Iterator instead of a Vec? Sounds interesting! \$\endgroup\$ – zachwhaley Aug 3 '18 at 15:38
  • 1
    \$\begingroup\$ About the first, I cannot summarize the ownership rules right now, but for this particular issue, if you hold a reference to a thing, you cannot move it because it'd invalidate the reference. You must clone it to have another item. And, yes, find should return an iterator. \$\endgroup\$ – Boiethios Aug 3 '18 at 15:56
  • \$\begingroup\$ @zachwhaley Result in main was introduced in 1.26. Here's the tracking issue if you're interested in the background. \$\endgroup\$ – Zeta Aug 4 '18 at 8:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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