Monday, January 26, 2015

No variable is infinitely better than an always true variable

When you are trying to simplify an architecture or remove dead code one of the juiciest finds you can come across is some variable or parameter which is always a fixed value. It is just crying out, "Remove Me", and I'm more than willing to answer. But it is never quite as easy as you think. Remember, it is still a variable which means its value, while clearly and logically fixed in the code in your window, might just have some way of being, well, variable.

Obscure Variables

So in my code base the most obscure types of variables are global boolean variables which can generally be set from the registry. The code that reads that registry in turn can be configured by anyone and I may or may not allow the value to be changed based on the name of the executable running. Sure, it has defaults, but they CAN be changed. But wait, who would change them right? I mean, they COULD change them, but I own the code, I own the application, I don't set the registry so they must be fixed... Right?? Think again. Someone, somewhere found your hidden gem and polished it for themselves.

There are other ways for variables to be obscure though. Maybe you marked them protected and someone decided to override and modify those values. Those are pretty easy to find, since the code is usually nearby. Unit tests can also do pretty much whatever they want. They can refuse to link in your version of the variable and instead link in their own, set to whatever value they want. The number of places you have to look in order to really verify your variable IS fixed is starting to become quite staggering.

No Variable is Better

So that variable was put into the code for a reason. Maybe it was paranoia. Maybe it was for a long forgotten feature that you never quite finished. Whatever the reason, that variable is going to ruin your life one day.

How do I know? Today I spent 8 hours tracking down how setting a bunch of if statements to true for a series of fixed variables in my code base could lead to a terrible, single block leak. And by single block, I mean exactly that. 16 bytes. 16 bytes in 8 different tests out of thousands of passing, non-leaking tests. They key, in this case was a registry controlled variable with some defaults. I had missed, that there was a configuration where a specific test host could carry the value false and get into the behavior I had just removed. It cost me, big, on a Sunday.

The Moral

No variable is infinitely better than an always true variable. Or a variable that is always one. Or a particular literal string. No matter how the variable is fixed, unless it is a locally scoped constant of some sort, you probably want to remove it from the code. You want to forget about all of those potential else clauses and you want to save others from shooting themselves in the foot by thinking your variable is in some way, well, variable. Its not. So don't make it one.

2 comments:

  1. Well, I guess a simple comment would have saved those hours...

    ReplyDelete
    Replies
    1. When an application has thousands of configurations possible, many wouldn't have known or understood the entire configuration of their test framework. The code in question (the point of change) was indeed commented as to its usage, however, the comment didn't fully detail all of the different applications and test frameworks which used it in that configuration. And if the new result of consumers of that configuration reaches 0, then you do indeed want to remove it to ensure it doesn't come back as dead code again later.

      Delete