A Minor WTF Code Snippet

This morning I was fixing a minor bug on the main site my team worked on a couple of months ago. We were getting a NullReferenceException, so I went into the code base and took a look at the offensive code. Well, as you can see, it was pretty smelly:

XmlNode productNode = null;

if(this.productDocument.SelectSingleNode(
    string.Format("products/product[@id='{0}']", productId)) != null)
{
    productNode = this.productDocument.SelectSingleNode(
        string.Format("products/product[@id='{0}']", productId));
}

if(productNode.SelectSingleNode("description") != null)
{
    // and so on...
}

The code looks up product information from a XML document via the product ID. The problem is pretty easy to see - if productNode is null, the second if statement will chunk. But what I found really whack what the first if statement. So...you check to see if that product element exists, and if it does, you redo the XPath query again! So I changed it to this:

XmlNode productNode = this.productDocument.SelectSingleNode(
    string.Format("products/product[@id='{0}']", productId));

if(productNode != nulll)
{
    XmlNode productDescriptionNode = productNode.SelectSingleNode("description");
    // and so on...
}

I know we all do bonehead coding implementations now and then. It's just funny (and embarrasing) to see something that is pretty ridiculous make it into production code. At least I didn't write this web page!

* Posted at 09.01.2005 07:24:52 PM CST | Link *

Blog History