CodeSOD: Legacy Switchout

About a decade ago, I attended a talk. The speaker made the argument that "legacy code" may have many possible interpretations, but the practical view was to simply think of legacy code as "code without unit tests". Thus, the solution to modernizing your legacy code was to simply write unit tests. Refactoring the code to make it testable would have the side effect of modernizing the code base, and writing tests would act as documentation. It's that easy.

Andrew is struggling with some legacy code right now. Worse, they're trying to integrate a large mountain of legacy code into a custom, in-house CI/CD pipeline. This particular pile of legacy code dates back to the mid-2000s, so everything in it is glued together via XML. It was some of that XML code which started failing when Andrew threw some unit tests at it.

It doesn't start that bad:

static bool IsNamespace(XName name){return name.LocalName == "xmlns" || name.NamespaceName == "http://www.w3.org/2000/xmlns/";}

Now, this is a silly method, and even .Net 1.0 had an XmlNamespaceManager for handling interacting with namespaces. Without knowing more of the overall context, maybe this method was actually useful? At the very least, this method isn't a WTF.

One thing I want you to note, however, is that this method does the sane thing, and simply returns the result of a boolean comparison. That method was written by the same developer, and checked in on the same day as this method:

static bool IsNewtonsoftNamespace(XAttribute attribute){switch (attribute.Value){case "http://james.newtonking.com/projects/json": return true;}switch (attribute.Name.NamespaceName){case "http://james.newtonking.com/projects/json": return true;}return false;}

Between writing IsNamespace and IsNewtonsoftNamespace, the developer apparently forgot that they could just return boolean expressions. They also apparently forgot- or perhaps never knew- what an if statement is supposed to do.

Andrew at least has the code building in their CI solution, but can look forward to many more weeks of fixing test failures by going through code like this.

[Advertisement] Ensure your software is built only once and then deployed consistently across environments, by packaging your applications and components. Learn how today!