Setup

Today I had to deal with a “show-stopper” bug for a project. Upon first glance it looked like the bug was just introduced in the latest development cycle. An area of the application that behaved normally prior to this round of development is now deviating from the spec, but only in certain scenarios. If it was introduced, that means I could have been the developer to introduce it, which is fine; we all write buggy code. I would just rather not be the developer introducing this bug. I tend to think my code is clean, and elegant. And of course, I was interested in how my tests missed this.

Reproduction

After getting a backup of the QA database, I was able to reproduce the issue. This was a relief, since on the outset, it looked like it could have been a concurrency issue. Instead, it was easy to track down via debugging. What it boiled down to was something analogous to the following code:

bool exists = list.Any(item => item.SomeType == "abc");

if (exists)
{
    return list.Where(item => item.SomeBoolean);
}

Context

At the time the code was written, if item.SomeType was “abc”, then item.SomeBoolean would be true. In this case, the converse was also true; if item.SomeBoolean was true, then item.SomeType was “abc”. The two properties were implicitly coupled together.

Analysis

The problem is that we now have logic that depends on the coupling. What the developer wanted to return, was the items whose “SomeType” property was “abc” (trust me). Since he knew that this was the same as looking at the “SomeBoolean” property, he depended on that property for the return statement. The implicit nature of this coupling was manifested as a set of assumptions in the mind of the original developer, that guided the logic behind his code. That was a mouthful, but take a moment to grok it; understand what it means for other developers. The code worked, at the time it was written. However, it is obviously brittle. It is not future-proof. Well, it’s the future now, and the logic no longer holds water.

In other words

To frame the issue in other words, imagine writing a software system that manages people. You have a requirement to group people by their hair color. For some unknown reason, the requirement mandates that the app only needs to support blonds and brunettes. If this is the case, then getting a list of all people that do NOT have blond hair, is equal to getting a list of all people that have brown hair. Any developer worth his salt would never use that logic though. Six months down the road, you might get a new requirement to support red-heads, or white-hairs, or purple-hairs, or no-hairs. This means you have to be aware of the assumptions you’ve previously made, and correct them. This is doable, but we shouldn’t need to.

Back to reality

The “show-stopper” was not far removed from this hypothetical scenario. Your code should never make assumptions. This is defensive programming 101. Maybe I should similarly not make assumptions about the quality of the codebase. There is a certain level of trust we need to have in our fellow programmers, and certain assumptions that we need to be able to make about a code base.