The Daily Parker

Politics, Weather, Photography, and the Dog

Code smells

A code smell happens when a piece of software code looks like there might be something wrong with it, but you can't quite tell what. You use the smell to figure out where the bad code is hiding. Martin Fowler has devotes an entire chapter of Refactoring to code smells.

Here is an example, from a class that returns configuration information:

public string Read() { ... }

public double ReadD() { ... }

public int ReadN() { ... }

public string ReadString() { ... }

What's wrong with this code? Several things:

  1. It does the same thing four times.
  2. Having two similar methods that appear to return exactly the same data raises a red flag, because you don't know from looking at them why they differ but you feel like they differ for a reason.
  3. Despite the code's appearance, it actually offers no guarantee that the data requested will be of the correct type, only that the data will have a particular type.
  4. if some random piece of code requires a data type it doesn't support, you'll have to add yet another method to the class, or change the returned data to suit what you need, neither of which makes a lot of sense.

This code came from an ancient, functional-procedural outlook in which the type of data returned actually matters. Object-oriented design cares more about what the data represents. In other words, this code is like multiple different filler tubes on your car, one for each grade of gasoline.

There's more. A little farther on, we find these methods:

public int ReadArray(string, ref String[]) { ... }

public int ReadArray(string, ref int[]) { ... }

public int ReadIntArray(string, ref ArrayList) { ... }
	

In all, there are 14 "Readtype" methods on this class, some that return the data sought, others that return a status indication and pass the data back through a by ref parameter.

And, of course, since this is in a fundamental class, none of these methods has fewer than 5 references to it in the application, and some (like ReadN) have 34, making refactoring unusually difficult. I'll post again when I figure out how I'm going to do that.

Comments are closed