Thursday, October 19, 2006

Something not to do in C#

private void WriteTraceLine(string txt, DateTime btime)
{
if(tracing == "On")
{
try
{
traceFile = new StreamWriter(@"c:\archiveTracing.txt", true);
}
catch
{
traceFile = new StreamWriter(@"c:\archiveTracing1.txt", true);
}
DateTime endtime = DateTime.Now;
TimeSpan elaspedtime = endtime - btime;
traceFile.WriteLine(txt + elaspedtime.Minutes.ToString() + " seconds: " + elaspedtime.Seconds.ToString() + " Date: " + DateTime.Now.ToString());
traceFile.Close();
}
else
{
if(traceFile != null)
{
traceFile.Close();
}
}
}


Don't do the same thing in a catch that you do in the try. The chances that it works the second time are slim. Besides, if there was enough of a chance for the first one to fail, this should at the least be in another try catch block. I know the two are opening different files, but that's not enough of an issue. It does cause problems of it's own when trying to track down issues. Even when logging is on I have to have both files open and go back and forth looking for specific issues.

Furthermore the function WriteTraceLine that this is in, is the only place that the variable traceFile is used, but it is declared to be at the class level. Finally, this ignores all built in tracing functionality in C# and relies solely on tracing off or on.

The filenames are not configurable, even though the application is using a custom config file. At the least that would allow us to relocate the files to a folder that could be shared out on the server, without having to recompile code.

Finally the Elapsed time that is being logged, is how long the function took to open the trace file. This is not an important issue, and is something completely separate from the actual informatoin that was sent to be logged in the first place.

No comments: