Monday, October 23, 2006

Something not to do in C# Part 2 Bad Code

After I posted the other day, I got into looking at the rest of that class. The entire applicaiton was a single class, but there were things all over that were just annoying to me. Rather than rant too much about the code, I'll let it stand on it's own merits.

I will however point out a few of the things I found the most annoying.
1. The WriteTraceLine function writes elapsed time. The way that is called everywhere writes the number of minutes that it takes t oopen the tracing file. Even a slow process takes less than a minute so this writes a zero on every line.
2. DateTime.Now is passed in to WriteTraceLine everywhere, but not before being assigned to another variable the line before it.
3. Although this code reads a config file, the location of the trace file is hard coded. This is a problem when the application is deployed to a server and runs on the C drive, while the log should be written to the larger partition of the D drive.
4. CheckDepartments has a catch and finally that do nothing.

using System;
using System.Xml;
using System.IO;
using System.Collections;

namespace AfniRemovalArchive
{

class Removal
{
Department currentLocalDepartment;
string tracing;
ArrayList localDepartments;
StreamWriter traceFile;

[STAThread]
static void Main(string[] args)
{
Removal rmv = new Removal();
rmv.GetXmlSettings();
rmv.CheckDepartments();
}

private void GetXmlSettings()
{
DateTime begintime = DateTime.Now;
WriteTraceLine("Get xml settings Begin: ", begintime);

string path;
XmlDocument doc;
XmlNode root;

//the path of the MailWatcher xml will be the location
//of the service EXE itself.
path = Path.GetDirectoryName(this.GetType().Assembly.Location.ToString());
path += @"\Archive.xml";
doc = new XmlDocument();

try
{
doc.Load(path);
}
catch(System.Exception ex)
{
begintime = DateTime.Now;
WriteTraceLine("Get xml settings: " + ex.ToString(), begintime);
}
begintime = DateTime.Now;
WriteTraceLine("Get xml settings path: " + path, begintime);

root = doc.SelectSingleNode("/Archive/localdepartments");
localDepartments = new ArrayList();
Department departmentObject;


foreach(XmlNode department in root.ChildNodes)
{
departmentObject = new Department();
foreach(XmlNode depart in department.ChildNodes)
{
if(depart.Name == "ldepartment")
{
departmentObject.departmentName = depart.InnerText;
}

if(depart.Name == "locallocation")
{
departmentObject.localLocation = depart.InnerText;
}
}
localDepartments.Add(departmentObject);

}


try
{
root = doc.SelectSingleNode("/Archive");
}
catch(Exception ex)
{
begintime = DateTime.Now;
WriteTraceLine("Get xml settings: " + ex.ToString(), begintime);
}
tracing = root.Attributes["Tracing"].Value;

begintime = DateTime.Now;
WriteTraceLine("Get xml settings End: ", begintime);

}

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



private void CheckDepartments()
{
try
{
foreach(Department department in localDepartments)
{
currentLocalDepartment = department;
RemoveArchive();
}
}
catch
{
}
finally
{
}

}


private void RemoveArchive()
{

string localPath = currentLocalDepartment.localLocation;

DirectoryInfo objLocalDirectory = new DirectoryInfo(localPath);
DirectoryInfo[] objLocalDirectories = objLocalDirectory.GetDirectories();

for(int a=0; a < objLocalDirectories.Length; a++)
{
DirectoryInfo[] objLocalBatches = objLocalDirectories[a].GetDirectories("*.ARCHIVE");
for(int l=0; l < objLocalBatches.Length; l++)
{
Directory.Delete(objLocalBatches[l].FullName,true);
}
}
}
}
}

No comments: