6
\$\begingroup\$

I need some opinion if my code below is proper for an async process. This code only reformats text. Please let me know if this implementation is not proper or needs some improvement.

static void Main(string[] args)
{
    Task.Factory.StartNew(() => ReadCharacter("File 1.txt"));
    Task.Factory.StartNew(() => ReadCharacter("File 2.txt"));
    Console.WriteLine("Main Task");         
    Console.ReadLine();
}

static void ReadCharacter(string inputFile)
{
    string result;
    using (StreamReader sr = new StreamReader(inputFile))
    using (StreamWriter sw = new StreamWriter(string.Format("C:\\Out--{0}",inputFile)))
    {
        Console.WriteLine("Opening file : {0}", inputFile.ToString());
        while (!sr.EndOfStream)
        {
            result = sr.ReadLine();
            sw.WriteLine(string.Format("{0} --> {1}",result,Task.CurrentId.ToString()));
        } 
        Console.WriteLine("Finish {0}", inputFile);
        sr.Close(); sw.Close();
    }
}
\$\endgroup\$
6
\$\begingroup\$

Well, you start two Tasks each reading and writing a file but not the same. So basically it should be ok. Some remarks:

  1. Task.Factory.StartNew returns a Task. You probably want to wait until the tasks you have started are finished before you quit from main which you can do with Task.WaitAll. Something along these lines:

    var task1 = Task.Factory.StartNew(() => ReadCharacter("File 1.txt"));
    var task2 = Task.Factory.StartNew(() => ReadCharacter("File 2.txt"));
    Console.WriteLine("Main - waiting for tasks to finish");
    Task.WaitAll(task1, task2);         
    Console.WriteLine("Finished");
    
  2. Your worker method ReadCharacter writes directly to the console. What happens if you want to run this code in a windows service process where this should be logged instead? So you should pass some kind of ILogger to the method and use that to output the status/log messages.

  3. You hard-code your output directory to C:\. This should really be passed in as destination folder.

\$\endgroup\$
3
\$\begingroup\$

If you want to wait until all tasks are finished, as @ChrisWue explained, you can create an even simpler one-liner solution using Parallel.For:

Parallel.For(0, 2, i => ReadCharacter("File " + i + ".txt"));

This is handy for those situations where you just want to use as many threads possible for a parallel operation, but need to wait until all of them are finished.

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

It may work in this case when you're operating on two different files, but as soon as you are working on the same file you will realize that this is not a "proper" implementation of an asynchronous operation.

In cases where you are doing I/O operations on the same file you need to set a lock to prevent race conditions, or you should use the asynchronous methods BeginRead, BeginWrite, etc, as stated here

  • There is also no need to invoke .close() on the Streams as the using(..) { } block will take care of that for you.
  • As you are only using your result variable within the inner-most using(..) { } block I would declare result just before your while-loop in order to minimize the scope.
\$\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.