3 RESPONSES comments
MARCH 11TH, 2008
By RYAN

Just a taste –

Since I’ve been bitching to Dan lately about the code I inherited, here are a few examples:

System.DateTime answer = today.AddDays(-1);
YesterdayTemp = answer.ToString("dd");
TodayTemp = today.ToString("dd");
string TodayMonth = today.ToString("MM");
string currlogdir = @"C:\Faxserver\Logs\";
int currHour = DateTime.Now.Hour;
int currMinute = DateTime.Now.Minute;
int currSecond = DateTime.Now.Second;
if (currHour == 00 && currMinute == 00 && currSecond == 00)
{
      if (File.Exists(currlogdir + "AutoFaxErrorCountLog_" + TodayTemp + ".txt"))
      {
           DateTime tempcreatetime6 = File.GetCreationTime(currlogdir + "AutoFaxErrorCountLog_" + TodayTemp + ".txt");
           string tempday6 = tempcreatetime6.ToString("dd");
           string tempmonth6 = tempcreatetime6.ToString("MM");
           if (TodayTemp != tempday6 || TodayMonth != tempmonth6)
           {
                File.Delete(currlogdir + "AutoFaxErrorCountLog_" + TodayTemp + ".txt");
           }
      }
      FaxCountLog(YesterdayTemp, lblFaxesTodayDisplay.Text);
      FaxErrorCountLog(YesterdayTemp, lblErrorsTodayDisplay.Text);
      lblFaxesYesterdayDisplay.Text = lblFaxesTodayDisplay.Text;
      lblErrorsYesterdayDisplay.Text = lblErrorsTodayDisplay.Text;
      TextlblDisplayTodayAdd("0");
      TextlblDisplayTodayErrorAdd("0");
      TextlblDisplayHourAdd("0");
      TextlblDisplayHourErrorAdd("0");
      FaxesHour = 0;
      ErrorsHour = 0;
      FaxesToday = 0;
      ErrorsToday = 0;
}
if (currHour != 00 && currMinute == 00 && currSecond == 00)
{
     FaxCountLog(TodayTemp, lblFaxesTodayDisplay.Text);
     FaxErrorCountLog(TodayTemp, lblErrorsTodayDisplay.Text);
     TextlblDisplayHourAdd("0");
     TextlblDisplayHourErrorAdd("0");
     FaxesHour = 0;
     ErrorsHour = 0;
}
if (ElapsedHours == 23 && ElapsedMinutes == 59)
{
     ElapsedDays += 1;
     ElapsedHours = 00;
     ElapsedMinutes = 00;
     ElapsedSeconds = 00;
}
if (ElapsedMinutes == 59)
     {
     ElapsedHours += 1;
     ElapsedMinutes = 00;
}
if (ElapsedSeconds == 59)
{
     ElapsedMinutes += 1;
     ElapsedSeconds = 00;
}
else
{
     ElapsedSeconds += 1;
}
if (ElapsedHours >= 0 && ElapsedHours <= 9)
{
     ElapsedHoursTemp = "0" + ElapsedHours;
}
else
{
     ElapsedHoursTemp = ElapsedHours.ToString();
}
if (ElapsedMinutes >= 0 && ElapsedMinutes <= 9)
{
     ElapsedMinutesTemp = "0" + ElapsedMinutes;
}
else
{
     ElapsedMinutesTemp = ElapsedMinutes.ToString();
}
if (ElapsedSeconds >= 0 && ElapsedSeconds <= 9)
{
     ElapsedSecondsTemp = "0" + ElapsedSeconds;
}
else
{
     ElapsedSecondsTemp = ElapsedSeconds.ToString();
}
blElapsedTimeDisplay.Text = ElapsedDays + " Day(s), " + ElapsedHoursTemp + ":" + ElapsedMinutesTemp + ":" + ElapsedSecondsTemp;
}

Replaced with:

int totalSeconds = DateTime.Today.TotalSeconds;
Stopwatch elapsedTime = new Stopwatch();
elapsedTime.Start();
public class Log {
     public void Rotate (FileInfo logFile)
      {
          if ((logFile.Exists) && (logFile.LastAccessTime.Date != DateTime.Today.Date))
          {
               logFile.Delete()
          }
      }
      /* snip */
}
public class Reset {
     public void Labels(bool day, bool hour)
          {
                lblFaxesYesterdayDisplay.Text = lblFaxesTodayDisplay.Txt;
           /*snip*/
           }
     public void Counts(bool day, bool hour)
     {
                /*snip*/
     }
}
Reset reset = new reset;
FileInfo fileErrorLog = new FileInfo(logdir + "AutoFaxErrorCountLog_" + DateTime.Today.Day + ".txt");
if (totalSeconds % 3600 == 0)
{
     if (totalSeconds == 0)
     {
          log.Rotate(fileErrorLog);
          reset.Labels(true, true);
          reset.Counts(true, true);
     }
     else 
     {
          reset.Labels(false, true);
          reset.Labels(false, true);
     }
reset.Dispose();
}
TimeSpan ts = elapsedTime.Elapsed;
lblElapsedTimeDisplay.Text = String.Format("{0} Day(s), {1:00}:{2:00}:{3:00}", ts.Days, ts.Hours, ts.Minutes, ts.Seconds);
}

No, I’m not using Delgates until I can decouple this:
AutoFax.Form1.btnCleanErrors_Click(object, System.EventArgs) AutoFax.Form1.btnRestartPrintService_Click(object, System.EventArgs)
AutoFax.Form1.btnStart_Click(object, System.EventArgs)
AutoFax.Form1.CheckRdy() AutoFax.Form1.CheckSaved()
AutoFax.Form1.ChooseThreads(int)
AutoFax.Form1.CreateNotAndRdy()
AutoFax.Form1.Dispose(bool)
AutoFax.Form1.ErrorOutSecondsNumberGet()
AutoFax.Form1.ErrorOutSecondsNumberSet(int)
AutoFax.Form1.FaxCountLog(string, string)
AutoFax.Form1.FaxErrorCountLog(string, string)
AutoFax.Form1.FaxLog(string, bool)
AutoFax.Form1.Form1()
AutoFax.Form1.Form1_Exit(object, System.EventArgs)
AutoFax.Form1.GetFaxCount()
AutoFax.Form1.InitializeComponent()
AutoFax.Form1.Main()
AutoFax.Form1.menuItemAbout_Click(object, System.EventArgs)
AutoFax.Form1.menuItemShutDown_Click(object, System.EventArgs)
AutoFax.Form1.OnChanged(object, System.IO.FileSystemEventArgs)
AutoFax.Form1.ProcessFile(string, bool)
AutoFax.Form1.ProgressIncrement(int)
AutoFax.Form1.Rdy()
AutoFax.Form1.read_ini_settings()
AutoFax.Form1.SendAlert(string)
AutoFax.Form1.SendEmail(string)
AutoFax.Form1.SendFax()
AutoFax.Form1.SendFaxToProgram(string, string)
AutoFax.Form1.Textbox1Add(string)
AutoFax.Form1.Textbox1Append(string)
AutoFax.Form1.TextbtnStartadd(string)
AutoFax.Form1.TextlblDisplayHourAdd(string)
AutoFax.Form1.TextlblDisplayHourErrorAdd(string)
AutoFax.Form1.TextlblDisplayTodayAdd(string)
AutoFax.Form1.TextlblDisplayTodayErrorAdd(string)
AutoFax.Form1.TextlblFaxesAdd(string)
AutoFax.Form1.TextlblQueAdd(string)
AutoFax.Form1.TextlblSendAdd(string)
AutoFax.Form1.timer1_Tick_1(object, System.EventArgs)
AutoFax.Form1.timer2_Tick(object, System.EventArgs)
AutoFax.Form1.timerHBT_Tick(object, System.EventArgs)

Who needs classes? That Form1 class can just get every method known to man.
More joy:

String rdydir = @"C:\RDY\INCOMING\";
String notdir = @"G:\NOT\";
String str1 = path.ToUpper();
String delim = rdydir;
String delim1 = "NOT.RDY";
String str2 = str1.Trim(delim.ToCharArray());
if ((File.GetAttributes(path) & FileAttributes.Normal) == FileAttributes.Hidden)
{
    File.Delete(path);
    FaxLog("deleted file: [" + path + "]", false);
}
else
{
File.SetAttributes(path, FileAttributes.Normal);
File.Delete(path);
FaxLog("deleted file: [" + path + "]", false);
}
String str3 = str2.Trim(delim1.ToCharArray());
String NotFile = str3 + ".NOT";
TextlblQueAdd("looking for: [" + NotFile + "]");

Replacement:

 string notdir = @"G:\NOT\";
string NotFile = Regex.Replace(path, @".*\\(\w+\d+).*", "$1" + ".NOT").ToUpper();

This is just…

string templine = sr.ReadLine();
sr.Close();
if (templine != null)
{
      int test = templine.IndexOf("[");
      int test2 = templine.IndexOf("]");
      string test3 = templine.Substring(test + 1, test2 - 1 - test);
      if (test3 != null)
      {
            Regex reDateTime = new Regex(@"^(\d{2})/(\d{2})/(\d{4}) (\d{2}):(\d{2}):(\d{2})$");
            if (reDateTime.IsMatch(test3))
            {
                  int test4 = test3.IndexOf(" ");
                  string test5 = test3.Substring(0, test4);
                  if (test5 == TodayMonthTempStr + "/" + TodayDayTempStr + "/" + TodayYearTemp)
                  {
                        filedatetimeisthesame = true;
                        // file exists and it has today's date, so just append to file
                        FileStream fs2 = new FileStream(templogpath + BeginLogName + MiddleLogName + TodayDayTempStr + ".txt", FileMode.Append, FileAccess.Write);
                        StreamWriter m_streamWriter2 = new StreamWriter(fs2);
                        string FaxLogDate2 = DateTime.UtcNow.ToUniversalTime().ToShortDateString();
                        string FaxLogTime2 = DateTime.UtcNow.ToUniversalTime().ToLongTimeString();
                        m_streamWriter2.WriteLine("[" + FaxLogDate2 + " " + FaxLogTime2 + "], " + FaxLog);
                        m_streamWriter2.Flush();
                        m_streamWriter2.Close();
                        fs2.Close();
                   }
              }
         }
    }
}

Replace:

if (fileAutoFaxLog.LastWriteTime.Date == DateTime.Today.Date) 
{  
      filedatetimeisthesame = true;
       // file exists and it has today's date, so just append to file
       FileStream fs2 = new FileStream(@"C:\Faxserver\Logs\AutofaxLog_" + DateTime.Now.ToString("dd") + ".txt", FileMode.Append, FileAccess.Write);
       StreamWriter m_streamWriter2 = new StreamWriter(fs2);
       m_streamWriter2.WriteLine("[" + DateTime.Now.ToString() + "], " + FaxLog);
       m_streamWriter2.Flush();
       m_streamWriter2.Close();
       m_streamWriter2.Dispose();
       fs2.Close();
       fs2.Dispose();
}

Lots of these (15 or so) in the code:

DateTime hbtTime = DateTime.UtcNow.ToUniversalTime();
string hbtMonth = hbtTime.Month.ToString();
if (hbtMonth.Length == 1)
{
     hbtMonth = "0" + hbtMonth;
}
string hbtDay = hbtTime.Day.ToString();
if (hbtDay.Length == 1)
{
     hbtDay = "0" + hbtDay;
}
string hbtYear = hbtTime.Year.ToString();
string hbtHour = hbtTime.Hour.ToString();
if (hbtHour.Length == 1)
{
     hbtHour = "0" + hbtHour;
}
string hbtMinute = hbtTime.Minute.ToString();
if (hbtMinute.Length == 1)
{
     hbtMinute = "0" + hbtMinute;
}
     string hbtSecond = hbtTime.Second.ToString();
if (hbtSecond.Length == 1)
{
      hbtSecond = "0" + hbtSecond;
}
string strHBTtime = hbtMonth + "/" + hbtDay + "/" + hbtYear + ", " + hbtHour + ":" + hbtMinute + ":" + hbtSecond;

Which is really:

string strHBTtime = DateTime.Now.ToString("MM/dd/yy HH:mm:ss");

This isn’t mentioning all the other weird shit in the code, like queues are always:

object whythefuck = queue.Peek();
string eh = whythefuck.ToString();
queue.Dequeue();

Rather than:

string morereadablelesswaste = queue.Dequeue().ToString();

Flags being set in 1,000 places to do the same thing. Every catch is handled the same way, but lots of copy+paste code rather than passing the Message to a method to handle it (better to just repeat ad-nauseum). Re-inventing booleans with lower performance by setting strings to “true” or “false” then comparing those (they never have any other value). No Dipose() or Finalize(). Ever. No comments. Pointless variable names that mean nothing, like tmpStr14. That also brings up: Hungarian notation. Gods, why use it in a strongly-typed language?

What started off as a 20 minute “add a method to email me” is going to be a “rework 4500 lines of code into (probably) 2000, by cleaning up the trash everywhere then adding classes and delegates.” Joy.

Tags: , ,

categories Bug, General