Friday, July 25, 2008

Remove Methods That are Not Called

[originally posted 5/9 on "Dev Blog"]

As we write lots and lots and lots of code, and our codebase expands beyond mere kilobytes and megabytes to the gigabyte range, it’s important to keep code bloat under control. One of the easiest ways to keep our codebase expansion in check is to remove code that is no longer in use. This usually means removing methods that are not called – though sometimes you can find entire classes to remove.

There are a handful of important reasons to remove old code. There are the general, feel-good reasons:

  • Less code is easier to maintain
  • Less code means faster searches
  • Clean code is much easier to read, and that saves us all a lot of time

Then there are the practical reasons:

  • Unused code can have bugs in it, and if someone were to somehow come across those bugs, and try to fix them, they are unwittingly wasting a great deal of time.
  • Often we make global changes to implementation details (for example, we change how we access our database, or we change how we raise and log errors). If a developer is making changes to an entire application, then any time spent on unused code is wasted time.
  • Upgrades to an application often require that all of the methods in the application be reviewed for possible updates. Any unused methods will cause another big fat waste of time.

The first step in removing unused code is finding the code to remove. The easiest way to find it is to pay attention to the methods that you are rendering obsolete as you are writing code. For example, imagine you have a method with the signature ValidateUser(string username, string password), and you decide to replace it with a new method with the signature ValidateUser(string username, string password, int applicationID). Once you’ve written the new method, and then replaced all calls to the old method with calls to the new method, you are now in a perfect position to remove the old method completely. You know it’s not being called from anywhere, because you just removed all calls to it. In fact, if it’s called from somewhere else, that’s a bug that you need to fix right away. This situation is the best of all possible opportunities to solve the unused method problem: if you remove code just as soon as it is rendered obsolete, you won’t introduce additional unused methods.

Unfortunately, sometimes unused code slips through the cracks, and gets left behind. The next best time for finding that code is when you are working in the code, whether you’re implementing new features or updating old ones. As you become familiar with an application, you get to know what code is used and what it is used for. The developer most familiar with a section of code should know what each method is used for, and should have a sense of what is not being used at all. If you come across code in one of your projects that you know has been left in when it should have been removed, remove it right away. It doesn’t take long, and it will save you and your teammates time in the future.

Of course there’s always that last, dreaded occasion for finding unused methods: when you are handed a project that was written by someone else, and asked to adopt the project as your own. You get someone else’s code, with no idea which methods are used, and which are not. Many developers would cringe at the thought of combing through an entire project, looking for unused methods. After all, it’s tedious and thankless: no one ever gets a big bonus for the code they deleted. But remember that every unused method represents countless wasted hours in the future. You can’t really afford not to find them and remove them.

In this situation, I haven’t found a better method than a simple search and destroy. I walk through every single method in a new project, search for its name in the rest of the application’s code, and delete it if I don’t find any. It takes a long time. But again, it’s nowhere near as much time as I would otherwise spend maintaining, updating, and otherwise babysitting completely unused code.

There are only two reasons I can think of for not removing unused methods:

  • We might need the methods in the future
  • The methods might be called from outside the application

The first of these methods may or may not be true. It’s hard to say what we’ll need in the future. But in a source controlled environment, the point is moot. We can always go back to source control and pull a copy of the deleted method. Deleted code doesn’t die: It just retires to VSS.

As for methods being called from outside an application, this may be a legitimate concern, depending on how our applications interact with each other. Methods that are called from outside an application should be clearly marked as such, with a continually updated listing, if possible, of the applications that call them. We need to be able to delete unused code, so any method which is not marked as being called from outside the application must be treated as a candidate for deletion.

Unused methods are a natural product of code improvement. As our applications are developed over time, new methods are introduced, existing methods are updated, and eventually some methods fall by the wayside. But leaving unused methods behind creates a mighty clutter, and wastes all of our time.

Ss.

No comments: