Cleaning up after yourself

Friday 20 August 2004

I’ve long been an advocate of deleting code and letting go of the past, but don’t leave mysterious garbage behind. Recently, a co-worker was removing some unneeded code. A certain class (let’s call it COldThing) was obsolete, and all references to it had to go. He changed code that looked like this:

static bool bInitted = false;

if (!bInitted) {
    gpOldThing = new COldThing();
    bInitted = true;
}

to something that looked like this:

static bool bInitted = false;

if (!bInitted) {
    bInitted = true;
}

This is just inexcusable. Why leave the empty initialization logic? The only explanation is simple haste. I ran across this code while working in the file, and it took some digging to figure out why this no-op initialization logic was there.

I added a few paragraphs to Deleting Code about this.

Comments

[gravatar]
Chad Walstrom 10:19 AM on 20 Aug 2004

If you know you're going to be removing a callable at some point in the future, why not provide deprecation warnings to the programmer? For example, python uses the following code:

import warnings
def callableFunction(arg1, arg2, arg3=None):
  ...
  if not arg3 == None:
    warnings.warn("arg3 argument is deprecated", DeprecationWarning)
  ...

Once the code is slated to be removed, raise an exception instead.

class DeprecatedException(Exception):
  pass
def callableFunction(arg1, arg2, arg3=None):
  ...
  if not arg3 == None:
    raise DeprecatedException("arg3 of callableFunction is deprecated!\n")
  ...

That way, you know for certain that your existing code has been cleaned up of all old references. At which point, it's up to you do decide whether to remove the deprecation warnings and exceptions. If you publish a library that other projects use, you should probably leave them in place.

Add a comment:

Ignore this:
Leave this empty:
Name is required. Either email or web are required. Email won't be displayed and I won't spam you. Your web site won't be indexed by search engines.
Don't put anything here:
Leave this empty:
URLs auto-link and some tags are allowed: <a><b><i><p><br><pre>.