Vince Ashby-Smith
Vince Ashby-Smith

Reputation: 1192

Most efficient way to return file list over multiple folders

Hi all i've got the following code which will go off and get all files from a route directory, including sub directories, when certain criteria are matched. Currently i have two methods; one to go get all the files and add them to a List and then another method to return the List. Just wondering if this is the best way to do it or is it more efficient to combine the two? Or can my code be re-written to be more efficient? Sorry i'm quite new to all this!

public class FileUtility
    {
        List<FileInfo> fileInfoList = new List<FileInfo>();

        public void ProcessDir(string sourceDir, String userName)
        {
            try
            {
                string userNameFirstLetter = userName.First().ToString();
                DirectoryInfo di = new DirectoryInfo(sourceDir);

                foreach (FileInfo fi in di.GetFiles())
                {
                    if (fi.Extension == ".xls" || fi.Extension == ".xlsx" || fi.Extension == ".pdf")
                    {
                        if (fi.Name.Contains(userName))
                        {
                            if (fi.Name.Contains("X"))
                            {
                                if(fi.Name.First().ToString().Equals(userNameFirstLetter))
                                {
                                    if (fi.Name.Split(Convert.ToChar("X"))[0].Equals(userName))
                                    {
                                        fileInfoList.Add(fi);
                                    }
                                }
                            }
                        }
                    }
                }

                // Recurse into subdirectories of this directory.
                string[] subdirEntries = Directory.GetDirectories(sourceDir);
                foreach (string subdir in subdirEntries)
                {
                    // Do not iterate through reparse points
                    if ((File.GetAttributes(subdir) & FileAttributes.ReparsePoint) != FileAttributes.ReparsePoint)
                    {
                        ProcessDir(subdir, userName);
                    }
                }
            }
            catch (DirectoryNotFoundException exp)
            {
                throw new DirectoryNotFoundException("Directory not found " + exp.Message);
            }
            catch (IOException exp)
            {
                throw new IOException("The Process cannot access the file because it is in use by another process " + exp.Message);
            }
        }

        public List<FileInfo> GetFileInfoList()
        {
            return fileInfoList;
        }
}

Upvotes: 1

Views: 1784

Answers (5)

agent-j
agent-j

Reputation: 27943

You probably don't need an instance of the class. Consider making it static instead, like this:

  public static class FileUtility
  {
    private static void ProcessDir(string sourceDir, String userName, List<FileInfo> fileInfoList)
    {
        try
        {
            string userNameFirstLetter = userName.First().ToString();
            DirectoryInfo di = new DirectoryInfo(sourceDir);

            foreach (FileInfo fi in di.GetFiles())
            {
                if ((fi.Extension == ".xls" || fi.Extension == ".xlsx" || fi.Extension == ".pdf")
                    && fi.Name.Contains(userName) && fi.Name.Contains("X")
                    && fi.Name.First().ToString().Equals(userNameFirstLetter)
                    && fi.Name.Split(Convert.ToChar("X"))[0].Equals(userName))
                 {
                    fileInfoList.Add(fi);
                 }
            }

            // Recurse into subdirectories of this directory.
            string[] subdirEntries = Directory.GetDirectories(sourceDir);
            foreach (string subdir in subdirEntries)
            {
                // Do not iterate through reparse points
                if ((File.GetAttributes(subdir) & FileAttributes.ReparsePoint) != FileAttributes.ReparsePoint)
                {
                    ProcessDir(subdir, userName, fileInfoList);
                }
            }
        }
        catch (DirectoryNotFoundException exp)
        {
            throw new DirectoryNotFoundException("Directory not found " + exp.Message);
        }
        catch (IOException exp)
        {
            throw new IOException("The Process cannot access the file because it is in use by another process " + exp.Message);
        }
    }

    public static List<FileInfo> GetFileInfoList(string sourceDir, string userName)
    {
        List<FileInfo> fileInfoList = new List<FileInfo>();
        ProcessDir(sourceDir, userName, fileInfoList);
        return fileInfoList;
    }
  }

Also, you can make it more concise by && ANDing the criteria together.

Upvotes: 0

Nick Udell
Nick Udell

Reputation: 2491

There's a much more efficient way to do it.

string[] filePaths = Directory.GetFiles(rootDirectory,searchPattern,SearchOption.AllDirectories);

You'll want to modify the searchPattern to cover your criteria for selecting files.

AFAIK the searchPattern operates similarly to a regular expression.

Alternatively you can get all the files that match certain file extensions and then loop through them to filter them by your criteria

Upvotes: 0

GvS
GvS

Reputation: 52528

If this is about getting a list of file, look at Paul Alan Taylor's answer.

If you want to learn (a bit) about recursion: return the list of files directly from the ProcessDir method.

Using the code you provide, needs at least to calls, and makes it error-prone. Specially if the class is reused.

I've change some things in your code:

  1. fileInfoList is declared inside ProcessDir
  2. the recursive call adds the result to "current" result.

    public class FileUtility {

        public List<FileInfo> ProcessDir(string sourceDir, String userName)
        {
            List<FileInfo> fileInfoList = new List<FileInfo>();
    
            try
            {
                string userNameFirstLetter = userName.First().ToString();
                DirectoryInfo di = new DirectoryInfo(sourceDir);
    
                foreach (FileInfo fi in di.GetFiles())
                {
                    if (fi.Extension == ".xls" || fi.Extension == ".xlsx" || fi.Extension == ".pdf")
                    {
                        if (fi.Name.Contains(userName))
                        {
                            if (fi.Name.Contains("X"))
                            {
                                if(fi.Name.First().ToString().Equals(userNameFirstLetter))
                                {
                                    if (fi.Name.Split(Convert.ToChar("X"))[0].Equals(userName))
                                    {
                                        fileInfoList.Add(fi);
                                    }
                                }
                            }
                        }
                    }
                }
    
                // Recurse into subdirectories of this directory.
                string[] subdirEntries = Directory.GetDirectories(sourceDir);
                foreach (string subdir in subdirEntries)
                {
                    // Do not iterate through reparse points
                    if ((File.GetAttributes(subdir) & FileAttributes.ReparsePoint) != FileAttributes.ReparsePoint)
                    {
                        fileInfoList.AddRange(ProcessDir(subdir, userName));
                    }
                }
            }
            catch (DirectoryNotFoundException exp)
            {
                throw new DirectoryNotFoundException("Directory not found " + exp.Message);
            }
            catch (IOException exp)
            {
                throw new IOException("The Process cannot access the file because it is in use by another process " + exp.Message);
            }
    
            return fileInfoList;
        }
    

    }

Note: Your code could be improved, for example by combining the if statements using "logical and" &&.

Upvotes: 0

Paul Alan Taylor
Paul Alan Taylor

Reputation: 10680

You don't need recursion.

string[] filePaths = Directory.GetFiles(@"c:\SomeDir\", "*.ext", SearchOption.AllDirectories);

Upvotes: 5

Chris Snowden
Chris Snowden

Reputation: 5002

You can do something like this:

DirectoryInfo dir = new DirectoryInfo(@"C:\PathName");
IEnumerable<FileInfo> files = dir.GetFiles("*.*");

Then you can use LINQ to filter this down if you're using C# 3.5 or above which I assume you are. Something like:

// Only use files with userName in the filename
files = files.Where(f => f.Name.Contains(userName));

Or combine the 2:

DirectoryInfo dir = new DirectoryInfo(@"C:\PathName");
IEnumerable<FileInfo> files = dir.GetFiles("*.*").Where(f => f.Name.Contains(userName));

Upvotes: 2

Related Questions