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();
}
}
3 Answers 3
Well, you start two Task
s each reading and writing a file but not the same. So basically it should be ok. Some remarks:
Task.Factory.StartNew
returns aTask
. You probably want to wait until the tasks you have started are finished before you quit from main which you can do withTask.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");
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 ofILogger
to the method and use that to output the status/log messages.You hard-code your output directory to
C:\
. This should really be passed in as destination folder.
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.
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 theusing(..) { }
block will take care of that for you. - As you are only using your
result
variable within the inner-mostusing(..) { }
block I would declareresult
just before your while-loop in order to minimize the scope.