Iterator to go traverse directory tree

While trying to showcase iterators I created an iterator that would go over a directory tree.

It would be nice to get some feedback for the code: both inlined and on this gist

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


//#[derive(Debug)]
//#[allow(dead_code)]
struct Walk {
    rds: Vec<ReadDir>,
}

impl Walk {
    fn new(root: &str) -> Result<Walk, Box<dyn std::error::Error>> {
        let path = Path::new(root);
        match path.read_dir() {
            Ok(rd) => {
                let w = Walk {
                    rds: vec![rd],
                };
                std::result::Result::Ok(w)
            },
            Err(err) => {
                println!("Could not open dir '{}': {}", root, err);
                Err(Box::new(err))
            },
        }
    }
}

impl Iterator for Walk {
    type Item = String;

    fn next(&mut self) -> Option<Self::Item> {
        if self.rds.len() == 0 {
            //println!("empty rds");
            return None;
        }
        let count = self.rds.len();
        let entry = self.rds[count - 1].next();
        match entry {
            Some(result) => {
                match result {
                    Ok(dir_entry) => {
                        if dir_entry.path().is_dir() {
                            match dir_entry.path().read_dir() {
                                Ok(rd) => {
                                    self.rds.push(rd);
                                }
                                Err(err) => {
                                    println!("Could not open dir {}", err);
                                }
                            }
                        }
                        return Some(dir_entry.path().to_str().unwrap().to_string());
                    },
                    Err(_err) =>  {
                        return None
                    },
                }
            },
            None => {
                self.rds.pop();
                self.next()
            }
        }
    }
}

fn main() {
    let args: Vec<String> = std::env::args().collect();
    if args.len() != 2 {
        eprintln!("Usage: {} PATH", args[0]);
        std::process::exit(1);
    }
    let path_from_user = &args[1];
    let tree = match Walk::new(path_from_user) {
        Ok(tree) => tree,
        Err(err) => {
            println!("Error: {}", err);
            std::process::exit(1);
        }
    };
    //println!("{:?}", &tree);
    for entry in tree {
        println!("{:?}", entry);
    }
}

1 Like

Try running Clippy. It only has a couple suggestions on your original code. :+1:

You can use Path and PathBuf instead of str and String. Including using args_os. Paths have a display method so you can print them if you don't want to go to the hassle of writing potentially-non-utf8 paths.


    fn new(root: &str) -> Result<Walk, Box<dyn std::error::Error>> {

I don't see any reason to box up the error here, so this could be

    fn new(root: &Path) -> io::Result<Self> {
        root.read_dir()
            .map(|rd| Walk { rds: vec![rd], })
            // This is here to preserve your `println` (should be `eprintln?`)
            .map_err(|err| {
                println!("Could not open dir '{}': {}", root.display(), err);
                err
            })
    }

        if self.rds.len() == 0 {
            //println!("empty rds");
            return None;
        }
        let count = self.rds.len();
        let entry = self.rds[count - 1].next();

Rust typically has shortcuts for patterns like this:

        let entry = self.rds.last_mut()?.next();

And you can remove a bit of match nesting by returning values needed to continue (this one is more a matter of preference):

        let current = self.rds.last_mut()?;
        let result = match current.next() {
            Some(result) => result,
            None => {
                self.rds.pop();
                return self.next();
            }
        };

Squints and seeing that closer to the top, I think I'd actually make this...

        loop {
            let current = self.rds.last_mut()?;
            let result = match current.next() {
                Some(result) => result,
                None => {
                    self.rds.pop();
                    continue;
                }
            };

This would probably be cleaner if that was factored into a separate method, but I didn't bother.


You're ignoring errors a lot around here. You could just print them like you do some times, or you could have an iterator over io::Result<PathBuf>, or some custom error with more context... but that can be a lot of work so I'm just going to preserve your behavior.

Here:

                        if dir_entry.path().is_dir() {
                            match dir_entry.path().read_dir() {
                        // ...
                        return Some(dir_entry.path().to_str().unwrap().to_string());

You're calling path a lot, which creates a new PathBuf each time. You just need to call it once.

            // `.ok()?` ignores the error (which is what you were doing)
            let dir_entry = result.ok()?;
            let path = dir_entry.path();
            if path.is_dir() {
                match path.read_dir() {
                    Ok(rd) => self.rds.push(rd),
                    Err(err) => println!(
                        "Could not open dir '{}': {}",
                        path.display(),
                        err,
                    ),
                }
            }

           return Some(path);

ExitCode exists, though frankly I don't know that it's really that much better in this case. You can also return Results straight out of main, but I typically don't, because the built-in error reporting for that uses Debug instead of Display.


This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.