Removing dead code

Ambulance_Stretcher What does your code terrain look like?  Are there bodies of dead logic lying here and there?  Maybe they helped briefly while you worked towards a better solution or perhaps they just fell victim to changing business rules.

At a recent Code Camp, there was a question about code generation and I answered that we (as developers) are required to love every line of code (it drew a laugh from many).  But love isn't cheap ... it takes time, care and hard work.  Therefore we should only love and care for the lines of code that we absolutely need.  Anything not being used should be removed.

 

 

 

 

 

What excuses are there to keep unused code?

  • We may need it in the future
    • The classic argument - the typical agile response being YAGNI. 
    • Keeping it in the code-base takes maintenance in the form of understanding it, updating associated tests and working around it to add other functionality.
    • It will always remain in source control should we ever need it again
  • If it ain't broke, don't fix it.
    • Bob Martin sums this up well when he defines what "broke" means:  Code should 1) work 2) be maintainable and 3) communicate its intent.  Clearly dead code violates 2 and 3.
    • This idea also defeats the goal of loving every line of code - leaving something alone for no good reason is neglect.

I just spent a good half hour cleaning up some code and mostly removing unused code.  It was annoying since it wasn't even being used but at least I am happier with the code now.  A good tool when investigating code you suspect may be dead is ALT-F7 using Jetbrains Resharper - you can easily find all usages of a method or class even in large code-bases.  A cheap alternative is to use "lean on the compiler" (as discussed by Michael Feathers) - this is simply changing a class name or method name by one character and try to compile (the compiler will then let you know what is depending on that code).

Jonathan Cogley is the CEO and founder of Thycotic Software, a .NET consulting company and ISV in Washington DC.  Our product, Secret Server is a enterprise password manager system for teams to secure their passwords.  Is your team still storing passwords in Excel?

4 Comments

  • When using C# it's a better idea to use

    [System.Obsolete("use class Namespace.Security.Password instead")]
    public class Password
    {
    }

    Then the compiler would state obsolete functions are being used, this might work better when cleaning up and not being sure if it's still used. (Also it's faster when cleaning up more than one function:))

  • Hmmm, good idea - thanks for the post.

    Our experience with Obsolete has been poor - usually they get thrown in and then stay in the code for weeks to months. I guess that is just down to discipline though.

    It makes sense to use it as a tool while cleaning up but try not to commit the Obsoletes.

    Disclaimer to readers: I am not saying to never commit Obsolete, just not if it is only being used as a clean up tool.

  • You can also use FXCop to report out unused code and variables.

  • I guess the use of ObsoleteAttribute depends on rather it's private code or a public API. In the case of a public API, ObsoleteAttribute is a necessity to provide backwards compatibility.

Comments have been disabled for this content.