Extracting files and directories from a provided list into separate lists

Question

This code gets a list of all files and directories from a specific location. Thes code below works.

List<string> FullList = new List<string>(LoadFiles("patch.txt"));
List<string> ShortCatalogList = new List<string>();
List<string> ShortFileList = new List<string>();
 for (int i = 0; i < FullList.Count; i++)
 {
 if (System.IO.Path.HasExtension(FullList[i]))
 {
 ShortFileList.Add(FullList[i]);
 }
 if (!System.IO.Path.HasExtension(FullList[i]))
 {
 ShortCatalogList.Add(FullList[i]);
 if (Directory.GetFiles(FullList[i]).Length > 0)
 { 
 string[] subFile = Directory.GetFiles(FullList[i]);
 for (int io = 0; io < subFile.Length; io++)
 {
 ShortFileList.Add(subFile[io]);
 }
 }
 if (Directory.GetDirectories(FullList[i]).Length > 0)
 {
 string[] subFolder = Directory.GetDirectories(FullList[i]);
 for (int io = 0; io < subFolder.Length; io++)
 {
 FullList.Add(subFolder[io]);
 } 
 }
 }
 }

I know that I need to protect from permission problem.

Answer 1

Most is said about this code in the other answer, but there is a simplification which can be used to beautify this code a lot.

The Directory class has two methods which returns IEnumerable<string> namely EnumerateFiles() and EnumerateDirectories().

The results of the calls to this methods can be used to call AddRange() on the ShortFileList and FullList.

Like so

List<string> fullList = new List<string>(LoadFiles("patch.txt"));
List<string> shortCatalogList = new List<string>();
List<string> shortFileList = new List<string>();
for (int i = 0; i < fullList.Count; i++)
{
 if (System.IO.Path.HasExtension(fullList[i]))
 {
 shortFileList.Add(fullList[i]);
 continue;
 }
 shortCatalogList.Add(fullList[i]);
 var files = Directory.EnumerateFiles(fullList[i]);
 shortFileList.AddRange(files);
 var directories = Directory.EnumerateDirectories(fullList[i]);
 fullList.AddRange(directories);
}

Answer 2

The code's indentation is pretty terrible. Keeping a good indentation is the key to keep maintainable code. With any good IDE, you can auto-format your code!

Also, considering the naming conventions, your variables should be camelCased, not PascalCased. So FullList -> fullList. And subFile holds multiple files, so you should pluralize it! subFile -> subFiles

You should import the System.IO instead of typing System.IO.Path.HasExtension all the time.

You do this using using System.IO; (if you didn't know already).

Instead of having :

if (System.IO.Path.HasExtension(FullList[i]))
{ }
if (!System.IO.Path.HasExtension(FullList[i]))
{ }

You should use the else statement :

if (System.IO.Path.HasExtension(FullList[i]))
{ }
else
{ }

As @Heslacher pointed out, you should use the AddRange method, it does the same thing as :

for (int io = 0; io < subFile.Length; io++)
{
 ShortFileList.Add(subFile[io]);
}

So well, that's less code! :)

You shouldn't call Directory.GetFiles(FullList[i]) twice. It's the most costly operation in your code, so you should call it only once! Same thing for Directory.GetDirectories(FullList[i]).

Instead of typing fullList[i] all the time, you should create a variable to hold it! Not that accessing an array is that expansive, but well, it'll help for readability.

So, let's check the final code :

List<string> fullList = new List<string>(LoadFiles("patch.txt"));
List<string> shortCatalogList = new List<string>();
List<string> shortFileList = new List<string>();
for (int i = 0; i < fullList.Count; i++)
{
 string path = fullList[i];
 if (Path.HasExtension(path))
 {
 shortFileList.Add(path);
 }
 else
 {
 shortCatalogList.Add(path);
 string[] subFiles = Directory.GetFiles(path);
 string[] subFolders = Directory.GetDirectories(path);
 if (subFiles.Length > 0)
 {
 shortFileList.AddRange(subFiles);
 }
 if (subFolders.Length > 0)
 {
 fullList.AddRange(subFolders);
 }
 }
}