For example, we can use delegation as follows.
This allows us to change the behaviour of
a.doSomething depending on the late-bound object
While indirection is indeed a powerful tool, we should be careful not to overuse it. I will show what I mean through an example that I will refactor in steps. Each refactor will strife to make the code cleaner and more maintainable. And finally, I will do a comparison of the before and after, and see why the “after” may not be the best.
Example: Traffic Lights
Consider a class that controls traffic lights and their various states.
(Example adapted from one in the Confident Ruby book.)
TrafficLight class has four main responsiblities.
- Keeps track of the current
- Can switch to another state using
signal()to turn on different lamps, and in some cases may ring a warning bell.
- Knows what the
nextState()is given the current state.
Here is an example of usage.
The first code smell is that there are switch statements inside
nextState. Why is
switch a smell?
Firstly, it makes the method behaviours static, therefore inflexible. This means that if
we ever needed to change their behaviours then we will have to add more
case blocks, which also
means that we cannot dynamically add or change cases during runtime.
Secondly, the structure of both switch statements look awfully similar. My gut tells me there is a new abstraction that is screaming to come out.
Refactor #1: Eliminate Switch Statements
The insight here is that we can group the color and next state data into state objects. Therefore, the responsiblity
of which colored lamp to turn on and the next state is removed from
TrafficLight (through delegation).
Looking good so far.
Now, the second code smell is the special case of calling
ringWarningBell() when we are signaling
States.CAUTION. Let’s create a
State base class that will handle the special
signal cases through polymorphism.
Refactor #2: More Delegation!
The idea here is that we can move all of the signalling logic to the
State class. This way we can take
further advantage of delegation and keep our
TrafficLight class nice and clean.
The refactoring so far has reduced
signal from 11 LOC to 1, and
nextState from 8 down to 1 LOC. Another
cool part is that a new domain concept has been capture via a new class
State. Looks good to me!
Or is it?
Too Much Indirection
If you compare before and after of the
TrafficLight class, I would argue that the former is much easier to
Reasonability of code is an often discussed topic in programming. But what does it actually mean?
Code is easy to reason about if its output can be easily predicted given some input. That is, I can predict what happens without having to expend a lot of effort tracing through it.
For example, referentially transparent functions with immutable objects are easier to reason about because I don’t need to run through all possible mutations that can occur in the system. I know that given the same input, I will always get the same output. I also know that the objects I pass into the function cannot be mutated, leading to behaviour changes.
Looking at the
signal() method from the initial example, I can tell you without much effort what it will do given
the different states. Can I say the same with the final example?
Scenario: What Happens When I Signal?
Let’s step through the same scenario with the initial code, and the final, refactored code.
- Initial state is
- I call
- What happens?
Let’s take a look at the initial method in question. Look for the numbers attached to each step below.
Constrast this with the final refactored code. The step numbers are out of order and might be hard to follow, but this is what happens with indirection in real code. ;)
The difference in scanning the initial example versus the final one is that the the former is completely contained in one method, and can be scanned from top to bottom. In the refactored code, we had to jump all over the place, and keep track of where we have jumped from.
Another thing to note is the the
signal method of
State is tightly coupled to the
implementation, so we did not create any meaningful abstraction layers either.
Does this mean the refactored code is necessarily worse? No, I don’t think so. And it’s not necessarily better either. It really depends on the requirements and personal preference. The refactored code might capture the state transitions and colour requirements in a more cohesive manner. It might even increase the ease of extension.
One thing is for certain, the refactored code is more complex than the original. What might look like clean code initially (small functions/methods, no switch statements, etc.) might not be desirable.
Identifying the Necessity of Indirection
Here are a couple of ways to help identify unnecessary or bad indirection.
Very Tightly Coupled Objects
If you notice that some objects always change together, then it might be a sign that you got the abstractions wrong the first time around. This can be done by analyzing git log for example.
Sometimes for the sake of keeping code clean and DRY, we introduce the wrong abstractions early on. These bad abstractions can lead to bugs, and make our code very hard to reason with. If it often better to leave code duplication alone until you arrive at the insight to refactor properly.
Too Many Object Interactions
A good abstraction allows you to focus in on the task at hand, without complecting it with all the other things going on in the system. So if you find yourself constantly having to load a bunch of objects in your head in order to reason about the system, then it might be another sign that something is amiss.
Acting On Bad Indirection
Once you’ve identified bad indirection, the best course of action is usually to undo the damage and inline the code. Some editors will help you inline your functions, variables, etc., with a quick command. Once inlined, it is much easier to see the bigger picture.
You can then decide whether you want to refactor further by creating better abstractions.
Indirection is a valuable tool. When used correctly, it allows us to create powerful abstractions that allow us to extend and maintain our system more easily. However, like most things of value, indirection comes at a cost. It increases complexity.
Does this mean we shouldn’t use indirection? Of course not! You will not get very far without it. You should think a bit before refactoring, and make sure you are actually making your code better.
If you end up with unnecessary indirection, don’t be afraid to inline your code again. This way you can more easily reason about your system as a whole. You may then get the insight needed for your next refactor.
Edit (2015-12-13): Fixed typos in code example.