Tuesday 3 October 2006

Continuing the sad trek through my client's abandoned application, today I have to figure out what this procedure does:

class ClsTimeing 
{

string regST="9:00 AM",  regET ="5:00 PM";

public ClsTimeing
{
	// Read regST and regET values from a database table (not shown)
}

public string Timeing(string sT,string eT)
{
	DateTime sTime = Convert.ToDateTime(sT);
	DateTime eTime = Convert.ToDateTime(eT);

	DateTime rsTime = Convert.ToDateTime(regST);
	DateTime reTime = Convert.ToDateTime(regET);

	TimeSpan ts = eTime - sTime;
	int EncTime = Convert.ToInt32(ts.TotalMinutes); 
	int bMin=0,rMin=0, aMin=0;
	if(sTime <= rsTime)
	{
		bMin = Convert.ToInt32((rsTime - sTime).TotalMinutes) ;
		if(EncTime <= bMin)
			return EncTime+"-"+rMin+"-"+aMin;	
	}
	if(sTime <= reTime)
	{  
		rMin = Convert.ToInt32((reTime - sTime).TotalMinutes) ;
		if(EncTime <= rMin)
			return bMin+"-"+(EncTime-bMin)+"-"+aMin;
		else
			rMin -= bMin;
	} 
	EncTime -= (rMin+bMin);
	aMin = EncTime; 
	if(EncTime <= aMin)
		return bMin+"-"+rMin+"-"+aMin;	 

	return bMin+"-"+rMin+"-"+aMin;
}

}

The first really bad sign is, of course, that the method (and class) name is misspelled. The second is that, even if it were spelled correctly, the name gives no indication of what the class or method actually does.

I can guess, by looking at which feature uses it, that the developer intended it to determine which portion of an appointment occurs before or after business hours, because different rates apply. So let's see if the method does what it purports to do.

First, the method converts its input parameters and configuration data from string to DateTime. All four values are completely under the application's control, so why are they not already DateTime values? Good question. From this you can guess how well the calling procedure and configuration data were thought out.

Next, it creates a TimeSpan of the total length of the appointment. Fine. On the following line it converts the TimeSpan.TotalMinutes value (a double) into an int. Why? Because the output will comprise three integer values, so I guess the developer thought it best to convert the value early.

I'm not going to step through the tortured logic that follows, but I will call your attention to a few more indications that the developer who wrote this was a few sectors short of a volume:

  1. The variables use an assortment of Hungarian, PascalCase, and camelCase names, without conveying what they actually mean;
  2. The procedure returns from within multi-level if blocks in three places;
  3. The return statements all have multiple boxing and string concatenation steps, in one case involving in-line arithmetic; and
  4. All of the returned strings really contain three values each, in a quasi-array delimited by hyphens. (Nothing in the calling method prevented this one from returning an actual array, possibly an array of int values, except the developer's incompetence.)

I wish this were an isolated example, but alas, all the code in the application has about this level of quality.

Search
Navigation
Categories
On this page....
Archives
<November 2008>
SunMonTueWedThuFriSat
2627282930311
2345678
9101112131415
16171819202122
23242526272829
30123456
Total Posts: 65
This Year: 1
This Month: 0
This Week: 0
Comments: 9
Blogroll
Contact me
Send mail to the author(s) E-mail RSS 2.0 Atom 1.0
Administration