Adding a feature to the crawler today, I ran across this line:

if (!(entry.IsGood && entry.LastStatus != StatusCodes.SpeculativeAdd))

Now what idiot would write something like that? Oh, wait. I’m the only one who’s ever worked on this program.

There are two serious problems with that conditional. First, it’s incredibly difficult to keep the truth table for that expression in my head. Every time I look at that expression, I have to figure out again what it means.

The second and more serious problem is that once I’ve fixed the meaning in my head, I have to figure out if that’s what I really meant to say. And that entails looking through other code and documentation. If I’m lucky I won’t lose my tenuous hold on the expression’s meaning before I tie all the ends together.

I should know better than this. I do know better than this. I cannot imagine what I was thinking when I wrote that expression. What bothers me is that it seems to work, as the code has been in production for months. But it’s going to take me 30 minutes or more to prove that it’s really doing what I want it to do. Believe me, it’s tempting to say, “Well, it seems to work,” and just go on.

The simpler expression is:

if (entry.IsGood == false || entry.LastStatus == StatusCodes.SpeculativeAdd)

The cognitive load is much lower. And, yes, I’m fully aware that a very loud contingent of programmers will scream bloody murder about my choice of entry.isGood == false rather than !entry.isGood. Get over yourselves. In my 45 or so years of writing computer programs for a living, I’ve discovered that I and many others find the former easier to read and understand, especially in unfamiliar code or when scanning code looking for the cause of an unexplained bug. Too many programmers miss that exclamation point on a quick read. Terseness isn’t always a benefit.

Keep your code simple. Otherwise the maintenance programmer who comes by six months later will not be happy with you. And it’s likely that you will be that maintenance programmer.