Dawn of the dead
One of the problems with the code base that we’re refactoring is that it’s full of dead code. This week we dealt with it…
Throughout the system there were swathes of code that were commented out, there was very little in the way of explanation as to why the code was not currently required and when it might be required again. It may have been old code that wasn’t required any more, or was broken or new code that wasn’t finished yet, or was broken. The original developer of the system has left and we would never be able to know why the code was commented out.
This kind of thing is often the result of haphazard debugging where code is added and removed until the current problem goes away. In a system that is stored in a source control system there is no reason for dead code to remain in the main source as it can always be retrieved from a prior version if required.
Ned Batchelder has some sensible things to say about deleting code; and as he says:
If you have a chunk of code you don’t need any more, there’s one big reason to delete it for real rather than leaving it in a disabled state: to reduce noise and uncertainty. Some of the worst enemies a developer has are noise or uncertainty in his code, because they prevent him from working with it effectively in the future.
Dead code makes the live code harder to work with as it complicates the system and makes it harder to work out what’s going on - this is possibly less so with modern colour coding editors that can show comments in a different colour to live code. The most insidious problem with dead code is that is encourages more changes to be made in the same way, soon there is more dead code than live code in the system.
Luckily dead code removal is an easy and relatively low risk refactoring and this week we spent some time removing it. Once that was done we started on the harder task of tracking down and destroying the living dead; zombie code that isn’t commented out but that doesn’t ever get called. In a framework or library then you would expect to have plenty of code that’s only ever called by your test harnesses and your users. In an application this is less likely to be the case. Unfortunately we have plenty.
Locating functions that are never called is reasonably easy if you build with browse information enabled. Working out if the functions should be kept or removed is harder, but as with dead code, the use of a source control system means that deletion isn’t necessarily a permanent action.
A more subtle hiding place for zombie code is code that relies on impossible return conditions from functions that it calls. The call site may test the return value from a function and handle failure but the function called may never return anything but success. Sometimes this is an obvious hard coding other times it’s harder to see as the called function may itself call other functions which can only ever return a subset of the expected values.
We had situations where a whole raft of code returned boolean values and the lowest level functions were hard coded to only return success. By converting these low level functions into functions with a void return we managed to eliminate masses of half hearted error handling code as we rose up the call stack. Like code that was simply commented out, this code would never be executed and so removing it was a relatively risk free endeavor.
Like comments that aren’t kept in sync during maintenance, code that’s not compiled, or code that’s not called will rot. Removing dead and zombie code makes the code base smaller. You have less to understand. You have less to maintain. You have less to test.
