Tuesday, May 09, 2006

Refactoring to eliminate duplication

As with a previous post, this may be obvious, but I think merits elucidation and discussion.
There are many cases where some code cries out, "refactor me!", but there never seems to be sufficient time. In the opinion of most leaders in the software community, however, we should make time; our efforts will always pay off.

So here's something commonly seen:

case PartyAnimal.Age of
 2..12:
 begin
  PutRightLegIn;
  PutRightLegOut;
  DoHokeyPokey;
  ShakeAbout;
 end;
 13..19:
 begin
  PutRightLegIn;
  PutRightLegOut;
  DoMacarena;
  ShakeAbout;
 end;
 20..25: //etcetera

We have some obvious repetition here that should be extracted into a method (wouldn't it be really cool if we had an IDE that would do this for us? It does, you say? Please teach me how; BDS gives me an error every time; VS2005 never did).

To emphasize why it should be extracted into a method:
  • When (note - not if) PutRightLegIn needs to change in every case, the change can be made in one place.
  • It makes for less code in the listing (and in the .exe). Less code to do the same thing is always a good thing. (1)
  • To quote Martin Fowler, "the essence of good design is ensuring that the code says everything once and only once." And better design makes for more efficient maintenance. (2)
  • It follows the DRY principle: Every piece of knowledge in the development of something should have a single representation. (3) The piece of knowledge in this case is RightLegIn, RightLegOut, X, ShakeAbout.

So, of course, our refactored case statement (after adding our new method, DoPartyDanceWith) looks like:

case PartyAnimal.Age of
 2..12:  DoPartyDanceWith( HokeyPokey );
 13..19: DoPartyDanceWith( Macarena );
 20..25: //etcetera


I'm hoping the next time each of us has to modify something that looks like this, there will be a greater itch to refactor, then modify.

Notes:
1) The Art of Unix Programming (and a zillion other places)
2) Refactoring
3) The Pragmatic Programmer (and many other places)

No comments: