I was reading Martin Van Aken’s great post on how dead code rots a codebase. To be clear up front, I completely agree with him and he makes some wonderful points. In previous teams that I’ve worked with, I’ve had this conversation countless times, which usually results in the forceful removal of both commented-out and unused code. However, in putting together supporting arguments for said conversations, I’ve encountered two main reasons for not removing such code.
One case for keeping commented-out code
I’ll borrow an analogy for commented-out code from Martin’s post (emphasis added):
Comments are supposed to be a trail, like little bread crumbs dropped by the previous programmers to help you find your way. Each of them is a sign, which means you should ponder them : how easy would it make for the person following you to follow this trail ? How straightforward is it ? Having no trail signs at all make the road difficult, but false or contradictory ones can be even worse.
False trail signs absolutely make a trail difficult to follow. However, there’s a case where having no trail signs pose a problem: if they don’t indicate a dangerous path that’s hard to back out from.
If a block of code seems like an obvious way to solve a problem, but has some serious repercussions, then it will be helpful to include such code and leave it commented out. However, there must be a comment that clearly says “WARNING: do NOT solve this problem with the below code, for reasons x, y, and z.” It would be terrible for a new developer to come along doing some hero refactoring and think that they’ve found a great way to simplify some code — only to introduce a critical bug that’s already been fixed.
One case for keeping unused code
When you maintain a completely internal codebase, there is no reason to keep code that’s no longer used. However, if you maintain a library that has an external API, removing code that you believe is no longer used may have a breaking effect on anyone who uses your API. Once you make a contract to have your API work a specific way, it becomes costly to change that. You will need to communicate the change to everyone who uses the API. Even if the method really is no longer used, you don’t want to prevent someone from using it in a perfectly valid way in the future. This is why you must always design your API with its usage in mind.
Typically, it’s a much better idea to mark the method as deprecated, and then communicate a schedule for it’s complete removal. But as you know, deprecated methods live in libraries for a very long time, so don’t get too hopeful for removing that unused code just yet. Even if you don’t have a legal obligation to tell users when you drop methods, you may end up angering a whole whack of people when you do it.
A related example for an internal library is a persistence layer. There may be a situation in which the Delete method is never called. Unless this operation is forbidden, then you should probably still include it, because it’s an important method to fill out the basic CRUD operations on data. (Of course, you can reduce the code maintenance issue by using an ORM system, but really, that’s just pushing the same issue on to someone else’s plate.)
No hard rules
Once again, this goes to show that there are no hard rules for maintaining code, and that it pays to be aware of how changes you make will affect both your users and your fellow developers.