Combining .txt files

Question

A guy at the company I work for needed a small application that would combine multiple text files into one, larger text file.

I wrote a console application for this. it seems pretty efficient, but I was wondering if there would be an even more efficient way of doing this.

It has 2 important functions, one that gets the files from a folder, where string input is the folder location:

static string[] getFiles(string input)
{
    DirectoryInfo dinfo = new DirectoryInfo(@input);

    FileInfo[] files = dinfo.GetFiles("*.txt");
    List<string> list = new List<string>();

    foreach(FileInfo file in files)
    {
        list.Add(input + @"\" + file.Name);
    }

    string[] arr = list.ToArray();
    return arr;
}

And of course the function that combines the files together, its input are the name of the file (string newName) and an array with the names of the files found in the folder by getFiles() (string[] files):

static void writeDump(string newName, string[] files)
{
    if (!File.Exists(newName))
    {
        using (StreamWriter sw = File.CreateText(newName))
        {
            for (int i = 0; i < files.Length; i++)
            {
                using (StreamReader sr = File.OpenText(files[i]))
                {
                    string s = "";
                    while ((s = sr.ReadLine()) != null)
                    {
                        sw.WriteLine(s);
                    }
                }
            }
        }
    } else
    {
        Console.Clear();
        Console.ForegroundColor = ConsoleColor.Red;
        Console.WriteLine("File already exists");
        start(); //start is called from the main function
    }
}

And because start(); might be confusing, I'll also add the main function here:

static void Main(string[] args)
{
    start();
}

How efficient is this and could it be more efficient?

Answer 1

        list.Add(input + @"\" + file.Name);

Seems a bit pointless: file.FullName would get you the fully qualified name without throwing information away and reconstructing it. In fact, that method could be simplified with Linq to

static string[] getFiles(string input)
{
    DirectoryInfo dinfo = new DirectoryInfo(@input);
    return dinfo.GetFiles("*.txt").Select(f => f.FullName).ToArray();
}

I also note that the .Net convention for the name would be GetFiles with an initial uppercase letter.

---

for (int i = 0; i < files.Length; i++)
{
    using (StreamReader sr = File.OpenText(files[i]))
    {
        string s = "";
        while ((s = sr.ReadLine()) != null)
        {
            sw.WriteLine(s);
        }
    }
}

Since you don't care about i you could simplify things with foreach; and the initial value of s is unnecessary, so you could have

foreach (var filename in files)
{
    using (StreamReader sr = File.OpenText(filename))
    {
        string s;
        while ((s = sr.ReadLine()) != null)
        {
            sw.WriteLine(s);
        }
    }
}

But now we get to two key points of the requirements which aren't explicitly stated:

1. If the files don't end with newlines, this code will insert newlines. That may or may not be intended, and it may or may not be desirable.

2. This code is using an Encoding to parse the bytes to strings, then using an Encoding to convert the strings back to bytes. The particular encoding used is implicit. This isn't particularly efficient, but it does have some benefits:

   • If the files were generated by Microsoft tools, they are quite likely to start with BOMs (even if they're UTF-8). In the nasty case that they mix UTF-8-BOM, UTF-8, and UTF-16 then you rely on the encoding conversion.
   • Even if the files are consistent, you're going to avoid the appearance of BOMs embedded in the text that a straightforward byte-by-byte concatenation would give.

   It also has at least one non-performance-related disadvantage:

   • Regardless of the encoding of the input files, the output file is likely to be UTF-8-BOM, which may be an undesirable side-effect if they were all UTF-8 or UTF-16.

If you wanted a straight byte-by-byte conversion then it would be more efficient to use

using (var strmOut = File.Create(newName))
{
    foreach (var filename in files)
    {
        using (var strmIn = File.OpenRead(filename))
        {
            strmIn.CopyTo(strmOut);
        }
    }
}

If you can guarantee that the input files are all UTF-8-BOM then it would be more efficient to use

using (var strmOut = File.Create(newName))
{
    foreach (var filename in files)
    {
        using (var strmIn = File.OpenRead(filename))
        {
            strmIn.Position = 3;
            strmIn.CopyTo(strmOut);
        }
    }
}

although that's not production-quality code (should check that there are 3 bytes and that they correspond to a BOM).

Answer 2

Some quick remarks:

• Method names should be PascalCase: getFiles, writeDump, start.
• Use descriptive variable/parameter names: input doesn't tell me anything. Ditto newName, especially since that looks like it is a path and not just a name.
• Don't do this: input + @"\" + file.Name. This is why Path.Combine() exists.
• Why use DirectoryInfo when Directory.GetFiles exists? Matter of fact, all of getFiles can be replaced by that!
• Why use for (int i = 0; i < files.Length; i++) when foreach(var filePath in files) would be so much easier and clearer?

Answer 3

Minor but you are creating an FileInfo[] files when you could just process them.

You could do the names and text in one loop.

Return List. The consumer can cast to an array if they need to.

public static void ListTextPlus(out List<string> fileNames, string outFile, string dirName = @"c:\temp\")
{
    fileNames = new List<string>();
    string line;

    DirectoryInfo di = new DirectoryInfo(dirName);

    if (di != null && di.Exists)
    {
        using (StreamWriter sw = File.CreateText(outFile))
        {
            foreach (FileInfo fi in di.EnumerateFiles("*.txt", SearchOption.TopDirectoryOnly))
            {
                fileNames.Add(dirName + @"\" + fi.FullName);
                using (StreamReader sr = fi.OpenText())
                {
                    //all text would be more efficient but line by line is lower memory
                    while ((line = sr.ReadLine()) != null)
                    {
                        if (!string.IsNullOrEmpty(line))
                            sw.WriteLine(line);
                    }
                }
                sw.WriteLine("");
            }
        }
    }
}