views:

292

answers:

4

I am not sure if I am doing this correctly or if my logic is correct.

I am trying to go down a folder structure delete files older than a certain number of days, this part I have correctly implemented, delete empty folders.

Can all this be done in one loop?
Where do I do the folder delete?

I want to delete empty folders up to 3 or 4 level down.

    private static void TraverseTree(System.IO.DirectoryInfo folder, double days)
    {
        Stack<string> dirs = new Stack<string>();

        if (!folder.Exists)
            throw new ArgumentException();

        dirs.Push(folder.FullName);

        while (dirs.Count > 0)
        {
            string currentDir = dirs.Pop();
            string[] subDirs;
            try
            {
                subDirs = System.IO.Directory.GetDirectories(currentDir);
            }
            // An UnauthorizedAccessException exception will be thrown if we do not have
            // discovery permission on a folder or file. It may or may not be acceptable 
            // to ignore the exception and continue enumerating the remaining files and 
            // folders. It is also possible (but unlikely) that a DirectoryNotFound exception 
            // will be raised. This will happen if currentDir has been deleted by
            // another application or thread after our call to Directory.Exists. The 
            // choice of which exceptions to catch depends entirely on the specific task 
            // you are intending to perform and also on how much you know with certainty 
            // about the systems on which this code will run.
            catch (UnauthorizedAccessException e)
            {
                Console.WriteLine(e.Message);
                continue;
            }
            catch (System.IO.DirectoryNotFoundException e)
            {
                Console.WriteLine(e.Message);
                continue;
            }

            string[] files = null;
            try
            {
                files = System.IO.Directory.GetFiles(currentDir);
            }
            catch (UnauthorizedAccessException e)
            {

                Console.WriteLine(e.Message);
                continue;
            }
            catch (System.IO.DirectoryNotFoundException e)
            {
                Console.WriteLine(e.Message);
                continue;
            }

            // Perform the required action on each file here.
            // Modify this block to perform your required task.
            foreach (string file in files)
            {
                try
                {
                    // Perform whatever action is required in your scenario.
                    System.IO.FileInfo fi = new System.IO.FileInfo(file);
                    Console.WriteLine("{0}: {1}, {2}", fi.Name, fi.Length, fi.CreationTime);

                    // Delete old files
                    if (fi.LastWriteTime < DateTime.Now.AddDays(-days))
                        fi.Delete();
                }
                catch (System.IO.FileNotFoundException e)
                {
                    // If file was deleted by a separate application
                    //  or thread since the call to TraverseTree()
                    // then just continue.
                    Console.WriteLine(e.Message);
                    continue;
                }
            }

            // Push the subdirectories onto the stack for traversal.
            // This could also be done before handing the files.
            foreach (string str in subDirs)
                dirs.Push(str);
        }
    }

Code is from MSDN.

A: 

There is almost the same question here:

http://stackoverflow.com/questions/1288718/c-how-to-delete-all-files-and-folders-in-a-directory

This is delete by name, but you could check other properties.

Shiraz Bhaiji
Thanks I did look at the post before posting my question.
Picflight
+3  A: 

A recursive approach would probably be cleaner.

private static void DeleteOldFilesAndFolders(string path)
{
    foreach (string directory in System.IO.Directory.GetDirectories(path))
    {
        DeleteOldFilesAndFolders(directory);

        // If the directory is empty and old, delete it here.
    }

    foreach (string file in System.IO.Directory.GetFiles(path))
    {
        // Check the file's age and delete it if it's old.
    }
}
John Fisher
Give me a recursive file system walker and I'll give you a file system that will overflow your stack.
plinth
@plinth Really? How deep is your file system? You really have files buried hundreds of folders deep?
John Fisher
Yes really. If you set your stack to n, I will embed n + 1 folders and give you a "reasonable" use case as to why it is the way it is. Years ago, I worked on Acrobat Catalog and we had to bullet-proof it against just those kinds of situations. Heap allocation is somewhat harder to implement, much harder to exhaust and easier to trap when resources run out. Which is the best solution in the real world?
plinth
Would you mind pasting one of these real-world paths with over a hundred folders deep? I've never encountered anything close to that. In fact, it wasn't that long ago that it was simply impossible to have a path string long enough in Windows.
John Fisher
A: 

Here is a more general solution to the problem which gives you a file system walker implemented non-recursively as IEnumerable.

for which your solution can probably be implemented as:

List<string> directoriesToDelete = new List<string>();
DirectoryWalker walker = new DirectoryWalker(@"C:\pathToSource\src",
    dir => {
        if (Directory.GetFileSystemEntries(dir).Length == 0) {
            directoriesToDelete.Add(dir);
            return false;
        }
        return true;
    },
    file => {
        if (FileIsTooOld(file)) {
            return true;
        }
        return false;
    }
    );
foreach (string file in walker)
    File.Delete(file);
foreach (string dir in directoriesToDelete)
    Directory.Delete(dir);
plinth
+3  A: 

Something that I notice about your code is that the dozens of lines of "mechanism" for walking the tree structure completely overwhelms the two lines of code that actually do the work. That makes it hard to read, understand, change, debug and maintain this code.

Here's what I would do instead.

You have only three high-level operations in your program: (1) get all the files, (2) filter to find the ones to delete, (3) delete each file. So write a program which does each of those in one statement.

For the first operation, I would factor out the mechanism above into its own function: a function which implements, say, IEnumerable, and all it does is keeps on yielding out information about files. It doesn't do anything with them; its only purpose is to keep spitting out file infos.

Once you have that mechanism then you can start writing queries on top of that sequence to attain the second operation. The third operation then follows directly from the second.

In short, the main line of your program should look something like this:

var allFiles = TraverseFolder(folder);
var filesToDelete = from file in allFiles where IsOld(file) select file;
foreach(var fileToDelete in filesToDelete) Delete(fileToDelete);

Is that clear?

Eric Lippert
+1 Yes, very clear. Thanks
Picflight