0

I am calling this (Compact Framework - this code runs on a Windows CE handheld device) method:

public static List<string> GetXMLFiles(string fileType, string startingDir)
{
    const string EXTENSION = ".XML";
    string dirName = startingDir; 
    var fileNames = new List<String>();
    try
    {
        foreach (string f in Directory.GetFiles(dirName))
        {
            string ext = Path.GetExtension(f).ToUpper();
            string fileNameOnly = Path.GetFileNameWithoutExtension(f);
            if ((ext.Equals(EXTENSION, StringComparison.OrdinalIgnoreCase)) && (fileNameOnly.Contains(fileType)))
            {
                fileNames.Add(f);
            }
        }
        foreach (string d in Directory.GetDirectories(dirName))
        {
            GetXMLFiles(fileType, d);
        }
    }
    catch (Exception ex)
    {
        MessageBox.Show(ex.Message);
    }
    return fileNames;
}

...like so:

List<String> XMLFiles = CCRUtils.GetXMLFiles(fileType, "\\");
MessageBox.Show(XMLFiles.Count.ToString());

...yet it's returning nothing (MessageBox shows "0"), even though there is a file that matches fileType and has an .xml extension.

Is it because there's something wrong with my GetXMLFiles() method? According to one cat here, my method is fouled up, and I should be adding to the generic list of string (fileNames) more often.

If he's right, I don't get that, though, because it seems to me that this is how the method should work:

(a) The first foreach loop looks at files below the root; if a match is found, it's added to the generic list of string
(b) The second foreach loop makes a recursive call to its method, once for every subdirectory on the device; step "a" occurs again for that directory, adding any matches to the generic list of string.

In this way, all directories are searched, and any matches found are added to the generic list of string (fileNames).

After the second foreach loop runs its course/handles all directories, control drops to the final line of code, which returns fileNames to the caller.

So, according to my grokking of it, I should get the match returned, but I'm getting nothing whatsoever.

If I'm wrong and Alan is right, where should I be adding the additional call to add/how should this method be reworked?

UPDATE

Calling it like this:

List<String> XMLFiles = CCRUtils.GetXMLFiles(fileType, "\\");

...doesn't work, but this way does:

List<String> XMLFiles = CCRUtils.GetXMLFiles(fileType, @"\");
Community
  • 1
  • 1
B. Clay Shannon-B. Crow Raven
  • 8,547
  • 144
  • 472
  • 862

1 Answers1

3

In your recursion, you are losing the files you find in the subdirectories. Capture the return filenames this way:

foreach (string d in Directory.GetDirectories(dirName))
{
    fileNames.AddRange(GetXMLFiles(fileType, d));
}

What is going on is that this line var fileNames = new List<String>(); creates a local variable called fileNames. You may think because your method is static, the variables inside the method are static. This is not the case. So, each time you call GetXMLFiles, a copy of this variable is created for each call.

Since fileNames is local to each call of GetXMLFiles, you need to return to the caller all the fileNames it finds and the caller needs to add those to the collection that is local to it.

Brad Rem
  • 6,036
  • 2
  • 25
  • 50
  • I still don't grok why this is happening, but that's okay if it works; do you mean add this line of code above the recursive call? – B. Clay Shannon-B. Crow Raven Mar 05 '14 at 00:54
  • @B.ClayShannon, Replace your line in the loop with my line. – Brad Rem Mar 05 '14 at 00:56
  • I see, thanks. It's late, I guess (that's *my* excuse, anyway). – B. Clay Shannon-B. Crow Raven Mar 05 '14 at 00:58
  • 1
    @B.ClayShannon I tried to tell you yesterday! You said you don't understand why it is happening. Consider, what if you removed your call to GetXMLFiles inside the loop? You would expect it not to find any files in the sub directories because the loop does nothing right? Well, GetXMLFiles RETURNS a list of files, right? You weren't even using that returned list before, therefore it does nothing. You have to append it to the results using AddRange for it to build the entire list of files recursively. – Alan Mar 05 '14 at 15:10
  • I'll take your word for it, but to really understand it, I would have to see it visually - with arrows or some such. Trying to step through it in my mind, my brain has a meltdown, and I end up more confused than ever. – B. Clay Shannon-B. Crow Raven Mar 07 '14 at 17:41