Filtering a recursive directory listing, discarding subfolders

I needed a function that would take a list of folders and removed all sub folders, so that only the top level folders stayed in the list.

For example, given the list:

c:\stuf\morestuff
c:\stuf\morestuff\sub1
c:\otherstuf
c:\otherstuf\sub1
c:\otherstuf\sub2

I wanted the list to be reduced to:

c:\stuf\morestuff
c:\otherstuf

So I came up with this solution:

// remove empty strings and sub folders
private static void CleanUpFolders(List uniqueFolders)
{
    uniqueFolders.RemoveAll(
        delegate(string curFolder)
        {
            // remove empty
            if (string.IsNullOrEmpty(curFolder))
                return true;

            // remove sub paths
            if (uniqueFolders.Exists(
                delegate(string s)
                {
                    if (!string.IsNullOrEmpty(s) &&
                        curFolder.StartsWith(s) &&
                        string.Compare(s, curFolder) != 0)
                        return true;
                    return false;
                } ))
                return true;

            return false;
        }               
    );
}

This seems to work (not very well tested though) but I was left wondering about some things:

• is there an issue with using variables inside anonymous methods that were declared outside?

• any potential issues with nested anonymous methods?

• any other issues or best practices worth mentioning?

is there an issue with using variables inside anonymous methods that were declared outside?

No, this is what anonymous methods are designed for! It is a very useful trick to keep up your sleeve. Read up on Closures. There are all sorts of things you can do with them.

Obviously there are issues with doing anything when you don't understand it fully, but the way you are using them in your code is what these things are all about!


any potential issues with nested anonymous methods?

Same thing.


any other issues or best practices worth mentioning?

Unless you are still using c# 2, the syntax has been simplified to use what is known as a lambda. Instead of using

delegate(string curFolder) { ..code.. }

you can just go :

curFolder => ..code..

As RemoveAll takes a Predicate, you can also lose the return key word. As long as the statement evaluates to True or False, it will take that as the return.

You have some code that is basically going :

if x == true
    return true
return false

This can be simplified to :

return x

With those two things, your code could be simplified to :

uniqueFolders.RemoveAll(
        curFolder => 

            string.IsNullOrEmpty(curFolder) ||
                   uniqueFolders.Exists( s=> 
                        !string.IsNullOrEmpty(s) &&
                        curFolder.StartsWith(s) &&
                        string.Compare(s, curFolder) != 0)

    );

Its a bit of a mouthfull. You may want to factor out a new method.

uniqueFolders.RemoveAll( curFolder => IsNotRootFolder(uniqueFolders, curFolder ) );

bool IsNotRootFolder ( uniqueFolders, curFolder )
{
   return string.IsNullOrEmpty(curFolder) ||
          uniqueFolders.Exists( s=> 
            !string.IsNullOrEmpty(s) &&
            curFolder.StartsWith(s) &&
            string.Compare(s, curFolder) != 0)
}

StackExchange Code Review Q#1225, answer score: 12