Thursday, August 11, 2005


Never Comment-Out Code

I work with a lot of "legacy code," meaning software that was written a long time ago by other people and which has undergone a lot of changes in its history as the requirements have changed and as different people have worked on it. There's a lot of stuff in there that I don't like, but I generally have respect for what other people have done.

However, one thing that drives me bananas is when I see a function like this:

void Application::DisplayErrors()
// This function is no longer used.  Errors are displayed by the ErrorHandler.
//  if ( ! errors.IsEmpty() ){
//    for ( int i = 0; i < errors.Count(); ++i ){
//      Display( errors[i] );
//      }
//    errors.Clear();
//    }

The "//" characters that begin the lines turn them into comments, lines that have no effect. In this example, every line has been "commented out," and a comment added informing the reader that the function doesn't do anything.

I hate it when people do this. If the function doesn't do anything, it should be deleted, not simply rendered harmless. Some programmers want to leave the old code there in case it might be needed again someday, or as some sort of historical record of how the code used to work, but cruft like this just makes programs harder to read and harder to modify.

The problem is not just that the useless function a waste of screen space and reader attention. Somebody reading another portion of the code might see a call to DisplayErrors() and assume that it will cause errors to be displayed. They may even add a call to DisplayErrors() when they implement some new functionality, and then be stumped for a while when errors don't get displayed.

If code isn't needed, have some guts. Delete it. Don't be afraid. You can always get the old code from the version-control system if you need it again. And nobody cares how the code used to work.

(By the way, the brace-indentation style in that example drives me bananas too. But that's a rant for another day.)

Comments: Post a Comment

<< Home

This page is powered by Blogger. Isn't yours?