A Positively Negative Meditation on Sloppy Code

A few years ago, my reaction to encountering code like this was very predictable. You could set your watch to it:

public XmlDocument FooBar()
{
    XmlDocument doc = new XmlDocument();
    doc.LoadXml("<foo/>");
    XmlElement bar1 = doc.CreateElement("bar");
    doc.AppendChild(bar1);
    bar1.SetAttribute("id", "1");
    bar1.SetAttribute("value", "bar1");
    XmlElement bar2 = doc.CreateElement("bar");
    doc.AppendChild(bar2);
    bar2.SetAttribute("id", "2");
    bar2.SetAttribute("value", "bar2");
    // ...
    XmlElement bar30 = doc.CreateElement("bar");
    doc.AppendChild(bar30);
    bar30.SetAttribute("id", "30");
    bar30.SetAttribute("value", "bar30");
    return doc;
}

The problem with that should be obvious to anyone, and—if The Daily WTF is any indication—I think this is a common mistake. Times have changed, however.

These days, if I had to name my single biggest issue in dealing with code written by others I would have to give an example like this:

public Hashtable Batman(Hashtable ht)
{
    // ...
}

The problem here is not as obvious to your average first-year programmer, but I would summarize the difference like this:

  • It’s not well-defined what is going in.
  • It’s not well-defined what happening inside the function.
  • It’s not well-defined what is coming out (and this may vary depending on #1).

While these kinds of problems are often found together, it seems to be possible to find code that avoids one pitfall but not the other.

The main factor in my attitude shift, you could say, would be what I am personally working on. When the first example bothered me the most, I was only responsible for enhancing functionality created by others and/or fixing bugs in code of the same quality. It was easy to cite examples of bugs that would not have occurred—or at least would have been easier to fix—if the cut & paste keys had not been so liberally employed.

I spend most of my time now gluing together components in the large, working on architectural things that many developers will never know or care about, and debugging oddball problems. With that background it’s easy for me to say that a cut-and-paster with a thoughtful interface definition has done a good (or even very good) job.

Lack of functional abstraction doesn’t seem as reprehensible to me as it once did. You can always encapsulate it in some functional abstraction. Many great books have been written in prison, and many great software packages have been put together from pieces of spaghetti code.

The second example (the chaos point with no type safety) is another matter. This portends systemic instability. When I find this with the intention of encapsulating-and-never-speaking-of-it-again, it’s very difficult or impossible to locate all of the dependencies. I will usually cut my losses and start from scratch after encountering a few “function Batmans.”

I admit that this insouciance towards Ctrl+C/Ctrl+V is entirely the byproduct of self-interest. It's not that I think it's a good idea, it's just that I can't muster the energy to get upset about it. If a passionate new programmer were to vocally find fault with it I would wholeheartedly agree.

Tags: , , ,

Leave a Reply