How can i improve my rust code organization?

[drodreo]

I'm writing a function that iterate over a specific directory and recursively it's sub-directory. it's seems very ugly since i have to handle every Result<T, E> for each calling. Is there any way can make it looks more cleaver? Such as combine all these Result<T, E> and handle them in one place for example.

fn check_dir(self: &Self, path: &Path) -> io::Result<()> {
  // ...
  fs::read_dir(path)?
    .map(|opt_el| opt_el.map(|el| el.path()))
    .for_each(|el| {
      if let Ok(i_path) = el {
        if i_path.is_dir() {
          if let Err(error) = self.check_dir(i_path.as_path()) {
            log::error!(
              "Failed to check dir {}: {}",
              i_path.to_str().map_or_else(|| "", |e| e), error
            );
          }
        } else {
          match self.check_file(i_path.as_path()) {
            Err(error) => log::error!(
              "Failed to check file {}: {}",
              i_path.to_str().map_or_else(|| "", |e| e), error
            ),
            Ok(file) => {
              match self.record_file_hash(file) {
                Err(error) => log::error!(
                  "Failed to compute hash from file {}: {}",
                  i_path.to_str().map_or_else(|| "", |e| e), error
                ),
                Ok(hash) => self.records.push(hash)
              }
            }
          }
        }
      }
    });
}

I want to avoid "for loop" in case the directory may store a very large number of file or sub-directories, if it's possible

[scottmcm]

There are a couple of patterns described here that might be helpful: Iterating over Results - Rust By Example

@drodreo

I want to avoid "for loop"

Why do you want to avoid a for loop? For non-trivial loop bodies -- and hashing a file is definitely non-trivial -- there's nothing at all wrong with using a normal for loop. Indeed, I'd say that a for loop is more idiomatic here -- for_each is more for small bodies after long chains.

[drodreo]

Thanks for your guidance, that references is very helpful!!!
I refactored the code, and it looks much better than before. I think i should post it out, thought it's might be useful to who are facing the same problem as me.

fn check_dir(self: &Self, path: &Path) -> io::Result<()> {
  // ...

  let items: Result<Vec<PathBuf>, _> = fs::read_dir(path)?
    .map(|opt_el| opt_el.map(|el| el.path()))
    .collect();

  let items = items?;
  let (dirs, files): (Vec<&Path>, Vec<&Path>) = items.iter()
    .map(PathBuf::as_path)
    .partition(|path| path.is_dir());

  files.iter().for_each(|file| {
    let result = self.check_file(file)
      .and_then(|el| self.compute_file_hash(el));
    match result {
      Ok(hash) => self.records.push(hash),
      Err(error) => log::error!(
        "Failed to compute hash from file {}: {}",
        file.to_str().map_or_else(|| "", |e| e), error
      )
    }
  }

  dirs.iter().for_each(|dir| {
    if let Err(error) = self.check_dir(dir)) { 
      log::error!(
        "Failed to check dir {}: {}",
        dir.to_str().map_or_else(|| "", |e| e), error
      );
    }
  });
}

[H2CO3]

There's still a lots of unnecessary iterator wrangling and back-and-forth between str and Path in your code. If you want to display a Path, why don't you just use Path::display(), for example?

Only being able to guess what the signature/purpose of your check_file() and compute_file_hash() functions should be, here's a simplified, cleaned up version of your code.

[drodreo]

Wow! that awesome! before your version, i've been trying to delete let items = items? from the code for some while. And i didn't even know there was such a function in Path