6
\$\begingroup\$

I have a folder which contains lots of files. I need to write C# code that will open and read and display the content of it. Is this efficient code or should something be changed?

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Security;
using System.Threading;
using System.Threading.Tasks;
using System.Text;

using System.Collections;


class Program
{
    static void Main()
    {
        var sw = Stopwatch.StartNew();
        ProcessRead().Wait();
        Console.Write("Done ");

        Console.WriteLine("Elapsed Time" + sw.ElapsedMilliseconds+"and"+sw.ElapsedTicks);
        Console.ReadKey();
    }




    static async Task ProcessRead()
    {

        string folder = @"Directory";

        string[] fileEntries = Directory.GetFiles(folder);
        int count = 0;

        foreach (string fname in fileEntries)

        {

            if (File.Exists(fname) == false)
            {
                Console.WriteLine("file not found: " + fname);
            }
            else
            {
                try
                {
                    count++;
                    string text = await ReadTextAsync(fname);
                    Console.WriteLine(text);
                }
                catch (Exception ex)
                {
                    Console.WriteLine(ex.Message);
                }
            }
        }

        Console.WriteLine(count);
    }

    static async Task<string> ReadTextAsync(string filePath)
    {

        using (FileStream sourceStream = new FileStream(filePath,
            FileMode.Open, FileAccess.Read, FileShare.Read,
            bufferSize: 4096, useAsync: true))
        {
            StringBuilder sb = new StringBuilder();

            byte[] buffer = new byte[0x1000];
            int numRead;
            while ((numRead = await sourceStream.ReadAsync(buffer, 0, buffer.Length)) != 0)
            {
                string text = Encoding.UTF8.GetString(buffer, 0, numRead);
                sb.Append(text);
            }

            return sb.ToString();
        }
    }
}
\$\endgroup\$
  • 6
    \$\begingroup\$ "efficient" by what metrics? Execution Time? Memory Usage? Maintainability and Extendability? \$\endgroup\$ – Kaz Jan 12 '17 at 11:28
  • 1
    \$\begingroup\$ @Kaz:Execution time,Scalability,Memory usage \$\endgroup\$ – vikk Jan 12 '17 at 11:30
  • 3
    \$\begingroup\$ Hey guys, I closed the other question as duplicate, even though that was really the first, because this one has a well-received answer and the other had nothing. Also, it seems the user accounts need merging, that's being taken care of as well. Thanks for raising the issues. \$\endgroup\$ – janos Jan 12 '17 at 14:28
14
\$\begingroup\$
  • You could use Directory.EnumerateFiles to allow processing of each path without loading all the paths to memory.
  • It is not required to check if the file exists because you just checked that (getting the path with GetFiles).
  • Instead of implementing your own ReadTextAsync, just use File.ReadAllText.
  • There is no need to use a new synchronization context (async call) for each file. If you want to process the files in the background, it is better processing all files in one single Task than using one task for each file. Remember that each context switch produces a little overhead.
  • If you want to use an async API that way, consider using ConfigureAwait(true) to avoid context switches:

    await sourceStream.ReadAsync(buffer, 0, buffer.Length).ConfigureAwait(true)
    

I suppose the following code will do the same faster and with less memory consumption:

static async Task ProcessRead()
{
    await Task.Run(() =>
    {
        IEnumerable<string> fileEntries = Directory.EnumerateFiles(@"Directory");
        int count = 0;
        foreach (string fname in fileEntries)
        {
            try
            {
                count++;
                string text = File.ReadAllText(fname);
                    Console.WriteLine(text);
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
            }
        }

        Console.WriteLine(count);
    }
}
\$\endgroup\$
2
\$\begingroup\$

No need to put all the file names in an array first. Just process them.

If you really want async it is supported by StreamReader and TextReader.

From DirectoryInfo to FileInfo you can open the file directly.

Line by line will have a lower memory usage.

public static void ListText(string DirName = @"c:\temp\")
{
    DirectoryInfo di = new DirectoryInfo(DirName);
    if (di != null && di.Exists)
    {
        foreach (FileInfo fi in di.EnumerateFiles("*", SearchOption.TopDirectoryOnly))
        {
            //Debug.WriteLine(fi.Extension);
            //Debug.WriteLine(fi.FullName);
            if (   string.Compare(fi.Extension, ".txt", true) == 0
                || string.Compare(fi.Extension, ".text", true) == 0)
            {
                using (StreamReader sr = fi.OpenText())
                {
                    string s = "";
                    while ((s = sr.ReadLine()) != null)
                    {
                        if(!string.IsNullOrEmpty(s))
                        {
                            Debug.WriteLine(s); 
                        }
                    }
                }
            }
        }
    }
}
\$\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.