The code for this article is written in C# (tested in VS2005)
I'm still catching up with my blog reading; a few days ago I read an article on latindevelopers: sorting objects with IComparer (code in English, comments and article in Spanish); it reminded me of a common mistake: the use of conditional statements in (not so obvious) loops (e.g. callbacks) which results in a type of (not so obvious) code duplication.
In Delphi this error is commonly seen in component development, specifically on the Paint event where they use if statements to decide which font to use, colors, etc, and what's worst, many times an instance (of whatever objects are required) is created on each iteration; all those objects, of course, don't change unless you change properties of the component!!, so those should be cached and kept on variables where they can just be reused; yes, code reuse doesn't just mean re-using classes, or refactoring code into methods, caching objects is a form of code reuse because it avoids conditional statements and places code calls directly.
Let's see the code from the original example, specifically the PersonComparer class, it has a constructor which takes the SortField and stores that on a variable that is later used on the Compare method
public PersonComparer(SortField sortField) {
this.sortField = sortField;
}public int Compare(object x, object y) {
if (!((x is Person) & (y is Person))) {
throw new
ArgumentException("Los objetos deben ser de tipo Person.");
}
switch (sortField) {
case SortField.Name:
return
Comparer.DefaultInvariant.Compare(
((Person)x).Name, ((Person)y).Name);
case SortField.LastName:
return
Comparer.DefaultInvariant.Compare(
((Person)x).LastName, ((Person)y).LastName);
case SortField.Age:
return
((Person)x).Age - ((Person)y).Age;
default:
throw new
ArgumentException("Tipo de ordenamiento desconocido.");
}
}Perhaps you don't see it very clear because the example doesn't use generics, let's see what we could do using generics (there is a TimeZone2, so why can't I have my PersonComparer2?):
public class PersonComparer2<T> : System.Collections.Generic.IComparer<T> where T:Person {
delegate int ComparerDelegate(T x, T y);
ComparerDelegate comparerDelegate;Now we have a delegate declaration, a variable of that type and we got rid of the SortField variable, let's see the constructor:
public PersonComparer2(SortField sortField) {
switch (sortField) {
case SortField.Name:
comparerDelegate = delegate(T x, T y) {
return x.Name.CompareTo(y.Name);
};
break;
case SortField.LastName:
comparerDelegate = delegate(T x, T y) {
return x.LastName.CompareTo(y.LastName);
};
break;
case SortField.Age:
comparerDelegate = delegate(T x, T y) {
return x.Age - y.Age;
};
break;
default:
throw new
ArgumentException("unknown sort method.");
}
}
Instead of caching the SortField variable, I'm caching the method that will be executed when the comparison takes place; finally, let's see the actual Compare method:
public int Compare(T x, T y) {
return comparerDelegate(x, y);
}
do you see it? the decision of what method needs to be used for the comparison gets executed only once (on the constructor), then we can call it directly, instead of asking the same question over and over again on each iteration.
Every time you see a conditional statement on a loop or a method that gets executed in some kind of loop (like paint events, callbacks, etc) consider using this technique, it will help you build more structured and faster code, also consider using arrays or dictionaries of methods.
The concept applies to programming in general, but the implementation may vary from language to language, the main idea is to
cache objects /
methods in some way so you don't have to repeat conditional statements to create objects or decide which code block to execute.
The full code for this example
can be found here, as always, play with it, learn from it, improve it.
