Find all files in directory and subdirectories while ignoring noncritical exceptions

Question

I need to get a list of all files supported by some libary (images) in a specified directory and all subderictories. Root directory of drive might be chosen and in this case I want to skip system folders like "Windows" and "AppData" (some other?).

For such task I can use Directory.GetFiles() but problem is - it will stop after getting any exception. In this case I need to just ignore noncritical exceptions and skip folders that cause this exception.

This code looks reasonable to me. But I want to make sure that it is done right.

Please tell me if it should be changed somehow.

public static List<string> FindSupportedFiles(string root)
{
    // class from library 
    var optimizer = new ImageOptimizer();

    Queue<string> folders = new Queue<string>();
    List<string> files = new List<string>();

    folders.Enqueue(root);
    while (folders.Count != 0)
    {
        string currentFolder = folders.Dequeue();

        if (currentFolder.Contains(":\\Windows") ||
             (currentFolder.Contains(":\\Users") && currentFolder.Contains("\\AppData")))
        {
            if (folders.Count != 0) currentFolder = folders.Dequeue();
            continue;
        }

        string[] filesInCurrent = null;
        try
        {
            filesInCurrent = Directory.GetFiles(currentFolder, "*.*");
        }
        // Ignore this exceptions
        catch (UnauthorizedAccessException) { }
        catch (PathTooLongException) { }
        catch (DirectoryNotFoundException) { }

        foreach (string f in filesInCurrent)
            if (optimizer.IsSupported(f)) files.Add(f);

        string[] foldersInCurrent = null;
        try
        {
            foldersInCurrent = Directory.GetDirectories(currentFolder);
        }
        // Ignore this exceptions
        catch (UnauthorizedAccessException) { }
        catch (PathTooLongException) { }
        catch (DirectoryNotFoundException) { }

        foreach (string _current in foldersInCurrent)
            folders.Enqueue(_current);
    }
    return files;
}

Answer 1

You have multiple problems here:

• For the case that an exception is thrown and swallowed you get a NullReferenceException at iterating over the filesInCurrent.

• If the method would be called using C:/Windows/ (which is a valid path although it uses a slash instead of a backslash) the if condition would never result in true.

• The if (folders.Count != 0) currentFolder = folders.Dequeue(); in addition with the continue can lead to problems. Assume the next folder in the queue would be a valid folder like C:\XFiles by calling Dequeue() and continue the folder would silently be swallowed.

  ---

• Fixing the NullReferenceException is the easiest task. Just put the iterating over the filesInCurrent and foldersInCurrent inside the try block. By doing this you could place the declaration into the try as well.

• Instead of checking if the currentFolder.Contains() a not wanted path, you should just refuse to add the paths while iterating over foldersInCurrent.

• Instead of having the optimizer.IsSupported(f) which had former been a SomeLib.IsSupported() you should pass a Action<string, bool> which will just handle this. This will make your method future proof if e.g. the decision will be made to use a different lib.

• To overcome the / vs \ problem you should create a DirectoryInfo() object using the root as parameter. By enqueueing the FullName property of the DirectoryInfo object any found subdirectory will have the \ as path separator.

• The code should take advantage of the List<T>.AddRange() method instead of iterating over the files and adding them one by one.

• If you later decide to exclude different paths it would be good to extract checking of the validness of the folders to a separate method which could be passed as Action<string, bool> parameter as well.

• Parameters of public methods should always be ckecked for validness, hence a check for null and for emptiness of the root should be implemented.

• One could use the var type if the right hand side of the assignment makes the type obvious.

• Ommiting braces {} although tehy might be optional can lead to hidden and therfor hard to find bugs. I would like to encourage you to always use them.

• Underscore prefixed variablenames are usually only for class-level variables.

• Passing a root which doesn't exists should be checked at the beginning of the method as well. By returning early you won't need to catch a DirectoryNotFoundException.

---

Implementing the mentioned points will lead to

public static List<string> FindSupportedFiles(string root, Func<string, bool> isFileValid,Func<string,bool> isFolderValid)
{
    if (root == null) { throw new ArgumentNullException("root"); }
    if (string.IsNullOrWhiteSpace(root)) { throw new ArgumentException("The passed value may not be empty or whithespace", "root"); }

    var files = new List<string>();


    var rootDirectory = new DirectoryInfo(root);
    if (rootDirectory.Exists() == false) { return files; }

    //overcome problem about slash and backslash with using Contains()
    root = rootDirectory.FullName;
    if (isFolderValid(root) == false) { return files; }

    var folders = new Queue<string>();
    folders.Enqueue(root);
    while (folders.Count != 0)
    {
        string currentFolder = folders.Dequeue();

        try
        {
            var currentFiles = Directory.EnumerateFiles(currentFolder, "*.*").Where(f => isFileValid(f));
            files.AddRange(currentFiles);
        }
        // Ignore this exceptions
        catch (UnauthorizedAccessException) { }
        catch (PathTooLongException) { }

        try
        {
            var currentSubFolders = Directory.GetDirectories(currentFolder).Where(f => isFolderValid(f));
            foreach (string current in currentSubFolders)
            {
                folders.Enqueue(current);
            }
        }
        // Ignore this exceptions
        catch (UnauthorizedAccessException) { }
        catch (PathTooLongException) { }

    }
    return files;
}

private static bool IsFolderValid(string path)
{

    return (path.Contains(":\\Windows") ||
             (path.Contains(":\\Users") &&
              path.Contains("\\AppData"))) == false;
}  

and could be called like so

var optimizer = new ImageOptimizer();
var supportedFiles = FindSupportedFiles(path, optimizer.IsSupported, IsFolderValid);