4
\$\begingroup\$

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.

\$\endgroup\$
5
\$\begingroup\$

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);

}
\$\endgroup\$
2
\$\begingroup\$

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);
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.