Removing Redundancy Badly

We all quickly agree that removing redundancy is good (aka OAOO, DRY, etc). But removing redundancy can be done well or can be done badly.

The risk we run is that we can simply, mechanistically move repeated lines from two or more methods into a new method. This would remove the redundancy, but could obscure the design and make our classes harder to use.

The mechanistic movement of code only sees repetition, but can't see responsibility. Adjacent lines of code are adjacent primarily because they share temporal binding or temporal dependency, not because they necessarily compose a single responsibility. It is very important that every function have a single responsibility (reflected in the function's signature) so that the code can be understood, maintained, and refactored. If a refactoring to combine code along temporal dependency lines does not relocate code according to its responsibility, we get functions that are largely meaningless.

Let's say it this way code is always to be organized along responsibility lines.

A particularly nasty variation works like this:

  void function() {
      do_X();
      do_Y();
      do_Z();
  }

This method code is a string of functions that take no parameters and return nothing in the way of results. What it is really doing is setting the state of its class. You will find that do_Y has to be called after do_X, and do_Z has to be called after do_Y. They are bound temporally by dependency on the states of various member variables. Effectively, X, Y, and Z are states and do_X, do_Y, and do_Z are events that move the class between its states. In some cases, calling only do_X and do_Y will leave the class in a state that isn't useful.

In these cases, it looks reasonable to inline the code from do_* functions and then look for responsibility centers so that the code can be more intelligently divided.

Clearly, removing redundancy is good and important, but blindly moving any few repeated, adjacent lines to a method is hardly a good practice in itself.


!commentForm




timsed completed, returns 0