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?

I don't think it can be any more efficient then with using streams. It can only be prettier but for a quick&dirty tool this is quite good. — t3chb0t
– t3chb0t, Commented Jan 19, 2017 at 10:42
Cat also works in Windows. Sounds like you're already working from a command prompt. You/he could do cat path/to/files/*.txt > my_output_file.txt instead of writing a program. But writing programs is fun/learning ;-) — brian_o
– brian_o, Commented Jan 19, 2017 at 17:18
@Grey I just timed how long it takes to make a 1GB text file. 12 seconds for your program, 12 seconds for cat. I'm not trying to discourage you, but I don't think you should get the idea that using C# is going to result in way less time consuming operations as compared to basic shell commands. Unless you're addressing some kind of edge case (4 files that concatenate in less than a second probably doesn't qualify). — brian_o
– brian_o, Commented Jan 19, 2017 at 20:35

tugrul · Accepted Answer · 2017-01-19 22:40:45Z

        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:

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

thanks, I've adjusted (most of) my code to your suggestions, though I've left the file streams in, since the txt files are all UTF-8 BOM, and the file stream make it a bit easier for me to read, and isn't impacting the preformance that much, I think, might be wrong... and indeed, the convension for .NET is to use capitalcase functions, but I've always felt that was just for classes :) thanks for the suggestions, much apriciated — Grey
– Grey, Commented Jan 19, 2017 at 11:43

BCdotWEB · Accepted Answer · 2017-01-19 11:05:47Z

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?

paparazzo · Accepted Answer · 2017-01-19 17:11:38Z

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("");
            }
        }
    }
}

I considered saying something about this but didn't. IMO there is a slight documentation advantage to explicitly working with an eager sequence of input filenames, and that's that it avoids the need to think "If my output filename matches the wildcard, could anything go wrong?" — Peter Taylor
– Peter Taylor, Commented Jan 19, 2017 at 17:27
@PeterTaylor You also get a (very) slight advantage of .OpenText method — paparazzo
– paparazzo, Commented Jan 19, 2017 at 17:31

Stack Exchange Network

Combining .txt files

3 Answers 3

You must log in to answer this question.

Hot Network Questions

Combining .txt files

3 Answers 3

You must log in to answer this question.

Related

Hot Network Questions