Tuesday, December 6, 2011

I See Dead Code

I recently removed a lot of dead code from our code base at work. There were several similar directories, dead files, and unused code within live files.

All this unused code was a major nuisance for over a year. It would cause grep to show too much information -- most of it meaningless -- and it would even cause your IDE to jump to the old unused class file, rather than the one you actually wanted.
People were afraid to remove this code for fear of breaking things. After all, it was tied to our main shared library that could cripple over 200 of our websites at once. But enough was enough. I decided to embark on removing it. I have a lot of experience with removing dead code but rarely this high of a risk and with so much of it. I had only one day to figure it out, delete it, and test it. Otherwise, it would have to have be rolled back, for who knows how long, until anyone would attempt to remove it again.

Before removing code like this, I ask myself if it's good code that we might want to start using soon - or even right now under a configuration option. If it's useful code I wouldn't just delete it. Rather, I'd extract it out into classes that could become part of our library.

In my case it really was all just useless code. Some programmer from long ago had been a little too happy to copy and paste code to new files, leaving the old dead files lying around.

Some other code was living in live files but was effectively in a "dead block" because it would only happen under a condition that would never happen. I decided to remove this as well because such code will only confuse programmers every time they see it. In fact, I was about to spend time converting it to work with our new platform. What a waste of time that would have been! Fortunately, dead code like this is often easy to determine as dead, because you go to refactor it and can't figure out how to get it to run under that condition from the user interface. This is why it's we shouldn't rely solely on unit tests.

A coworker suggested that I keep the condition in the code at first, replacing the body of useless code with code to log a message such as "This condition should never happen." This way we could monitor the logs once it was deployed for a while, and really make sure the code wasn't being used by anything.

The easiest dead code to spot is commented-out code. Go ahead and remove this any time you see this. It will only sit there forever, no one will ever uncomment it, and if they really do need it that's what source control is for.

For those times that it's hard to tell if code is dead or not, the main tool I used to figure it out is grep. More specifically I use ack-grep. The key is to trust your grepping abilities and be extremely thorough. If you can prove that certain code isn't being referenced via other files, that's a huge step toward having the confidence to remove it.

Of course, it's not always that easy. Sometimes the code is referenced but that's okay; replacing it to use the proper files instead is certainly worth the effort.

Once you have removed the code, and all is working in your development sandbox, it's time to deploy it. It really isn't much different than writing new code or refactoring existing live code. Any time you commit code it could cause other things to break. This is why we do regression testing.

I highly recommend that you wait to push this kind of change out when there's no other important code that needs to be deployed too. If the "new" code is found to not be in a working state, then your code removal attempt will be the first source of blame and you will likely have to rollback and risk never getting to try again -- even if it wasn't the problem.

So go ahead and remove that dead code the next time you see it. You'll thank yourself later and so will your coworkers.