A few months ago I wrote a rant about unused pieces of code lying around the codebase. And, recently, we had to deal with it.

Some users were complaining about a bug which was rarely occurring, but was very annoying and was thus given high priority. It took a pair of developers two day-long attempts to finally pinpoint the root cause. And what it was? A part of an old method not intended to be ever executed (guarded by malfunct if statement) as it was replaced by new approach. It was left checked in “just in case”.

The fix was simple - just delete obsolete part of method.

Conclusion: Inactive code should be removed sooner rather than later.

Recently, I had a (somewhat heated) discussion with my colleagues about the value of inactive code. That is neutral name which emerged for what I think is harmful and damaging phenomenon. Large swathes of code which are either commented out, or, worse, not commented out but unreferenced from active code, or only referenced but otherwise unused.

To illustrate:

class SomeForgottenFeature { 
  private String mState = "";
  
  public void doSomething() {  
    // ...
  }  
}

class SomeActiveFeature {

  // this is the only mention of SomeForgottenFeature
  private SomeForgottenFeature mSomeForgottenFeature = new SomeForgottenFeature();

   // lots of useful stuff   
   // but no mention of mSomeForgottenFeature
}

So, on a few occasions, I had deleted such code.

Some of my arguments were:

• yes, the code was deleted, but it is still present in VCS history
• this is just a case of being tidy, that whatever code is unused should be removed so as not to burden each developer who looks into SomeActiveFeature
• these cases do add up and bring the project nearer to Death By A Thousand Cuts

Their argument was:

• yes, but what if at some point in future we need it again? It would be awkward to restore the functionality

My proposed flow for dealing with such case was:

1. search VCS for keywords, file names. For this to work best, it’s helpful to include as many keywords in the commit message
2. locate the revision where the feature was deleted
3. revert the commit, or just use the info in commit for the effort of reimplementing

Obviously, this effort would not be needed if the feature was not deleted in the first place. However, fact of the matter is that during the years, this inactive code was rarely updated with new APIs and paradigms, and was often left to its own devices with parts that needed change just commented out, waiting for better times, and reactivating it would require significant effort.

Another problem that was raised is that in order to search history, one needs to know that the class with feature even existed. Developer churn and people’s tendency to forget makes this a real issue in any project that lives for more than a year.

To tackle this, a list of removed features could be compiled and pinned to some shared (real or virtual) space. Or a file holding the list could be checked in and updated each time a large chunk of code is removed.

Conclusion

The takeaway here could be that whatever is obvious and expected to one party, may be opaque and nonobvious to the other. Therefore, when discussion starts to look like a conflict, one should always try to stay respectful and mindful of others.