It’s been a while since I’ve had to slice and dice legacy code. It reminds me how easily, non test driven code, slips into the abyss of tight coupling and the effort of retrofitting (unit) tests increasing in effort with time. Of course, I don’t believe TDD should be done all the time, but for most production code I do. In this entry, I’m going to use the Sprout Inner Class mechanism to demonstrate how to start making something that would have been untestable much more manageable.
The scenario
What we have below is the entry point into our program (i.e. a class with a main method). When I first encountered this class, it had too many responsibilities including, and not limited to, parsing command line options, some application logic, handling terminal signals, and wiring up all the objects for the program to start. Combined with an infinite working loop, you can imagine what a nightmare it was to unit test. Here’s the modified class, called RunningProgram representing the important parts of the class related to this walk through.
Our task: To reduce the number of responsibilities and test the body of the start method (we couldn’t simply extract method immediately due to the number of fields it modified)
package com.thekua.examples;
import sun.misc.Signal;
import sun.misc.SignalHandler;
public class RunningProgram implements SignalHandler {
private boolean running = true;
// some other fields
public void start() {
running = true;
while (running) {
// do some work (that we want to unit test)
// it changes about ten fields depending on what condition
// gets executed
}
// Finished doing some work
}
// it also had plenty of other methods
public static void main(String [] args) {
RunningProgram program = new RunningProgram();
Signal.handle(new Signal("TERM"), program);
Signal.handle(new Signal("INT"), program);
program.start();
}
public void handle(Signal signal) {
// Program terminated by a signal
running = false;
}
}
Step 1: Understand the dependencies that make this test difficult to test
To get some level of functional testing around this program we had a number of timed triggers, watching for state modifications with a given timeout. Our problem was infinity itself, or rather, not sure when to interrupt infinity to watch for when the body of work inside the loop had finished its work once, twice, and not too early. We could have made it more complicated by introducing lifecycle listeners yet we hesitated at that option because we thought it would complicate the code too much.
We noticed the use of the running flag. We noticed it was the condition for whether or not we continued looping, and was also the trigger for a graceful shutdown using the sun.misc.Signal class. Notice that the running program implements the SignalHandler interface as a result. We thought that running behaviour of the RunningProgram could be extracted into a separate aspect.
Step 2: Encapsulate, encapsulate, encapsulate
Our first task, was to remove direct access to the running flag since the class modified it in two places. Sprout an inner class, and simply delegate to getters and setters and we might find we have a class that looks like:
package com.thekua.examples;
import sun.misc.Signal;
import sun.misc.SignalHandler;
public class RunningProgram implements SignalHandler {
public static class RunningCondition {
private boolean running = true;
boolean shouldContinue() {
return running;
}
public void stop() {
running = false;
}
}
private RunningCondition condition = new RunningCondition();
// some other fields
public void start() {
while (condition.shouldContinue()) {
// do some work (that we want to unit test)
// it changes about ten fields depending on what condition
// gets executed
}
// Finished doing some work
}
// it also had plenty of other methods
public static void main(String [] args) {
RunningProgram program = new RunningProgram();
Signal.handle(new Signal("TERM"), program);
Signal.handle(new Signal("INT"), program);
program.start();
}
public void handle(Signal signal) {
// Program terminated by a signal
condition.stop();
}
}
Moving the running flag to a separate class gives us a number of benefits. It lets us hide the implementation of how we handle running, and puts us down the road of clearly teasing apart the overloaded responsibilities.
Step 3: Consolidate related behaviour
It bothered me that the main program had the shutdown hook. That behaviour definitely felt strongly related to the RunningCondition. I felt it was a good thing to move it to the that class. We now have something that looks like:
package com.thekua.examples;
import sun.misc.Signal;
import sun.misc.SignalHandler;
public class RunningProgram {
public static class RunningCondition implements SignalHandler {
private boolean running = true;
boolean shouldContinue() {
return running;
}
public void handle(Signal signal) {
// Program terminated by a signal
running = false;
}
}
private RunningCondition condition = new RunningCondition();
// some other fields
public void start() {
while (condition.shouldContinue()) {
// do some work (that we want to unit test)
// it changes about ten fields depending on what condition
// gets executed
}
// Finished doing some work
}
// it also had plenty of other methods
public static void main(String [] args) {
RunningProgram program = new RunningProgram();
Signal.handle(new Signal("TERM"), program.condition);
Signal.handle(new Signal("INT"), program.condition);
program.start();
}
}
Note that it is now the RunningCondition that now implements the SignalHandler interface (that we are using to register with the Signal
Step 3: Remove dependency chain
The difficulty with this class still exists. We cannot modify the RunningCondition of this program since it creates one for itself. Since I prefer Constructor Based Dependency Injection, I’m going to apply Introduce Parameter to Constructor, moving the field declaration to the constructor itself. Here it is what the class looks like now:
package com.thekua.examples;
import sun.misc.Signal;
import sun.misc.SignalHandler;
public class RunningProgram {
public static class RunningCondition implements SignalHandler {
private boolean running = true;
boolean shouldContinue() {
return running;
}
public void handle(Signal signal) {
// Program terminated by a signal
running = false;
}
}
private final RunningCondition condition;
public RunningProgram(RunningCondition condition) {
this.condition = condition;
}
// some other fields
public void start() {
while (condition.shouldContinue()) {
// do some work (that we want to unit test)
// it changes about ten fields depending on what condition
// gets executed
}
// Finished doing some work
}
// it also had plenty of other methods
public static void main(String [] args) {
RunningProgram program = new RunningProgram(new RunningCondition());
Signal.handle(new Signal("TERM"), program.condition);
Signal.handle(new Signal("INT"), program.condition);
program.start();
}
}
Note that we are still bound to the implementation of the specific RunningCondition, so it’s time to apply Extract Interface, and understand what role that RunningCondition has. We chose the name, RunStrategy for the role name, and to help keep the names more aligned, we ended up renaming RunningCondition to RunLikeADaemonStrategy. Our code now looks like this:
package com.thekua.examples;
import sun.misc.Signal;
import sun.misc.SignalHandler;
public class RunningProgram {
public static interface RunStrategy {
boolean shouldContinue();
}
public static class RunLikeADaemonStrategy implements SignalHandler, RunStrategy {
private boolean running = true;
public boolean shouldContinue() {
return running;
}
public void handle(Signal signal) {
// Program terminated by a signal
running = false;
}
}
private final RunStrategy runStrategy;
public RunningProgram(RunStrategy runStrategy) {
this.runStrategy = runStrategy;
}
// some other fields
public void start() {
while (runStrategy.shouldContinue()) {
// do some work (that we want to unit test)
// it changes about ten fields depending on what condition
// gets executed
}
// Finished doing some work
}
// it also had plenty of other methods
public static void main(String [] args) {
RunLikeADaemonStrategy strategy = new RunLikeADaemonStrategy();
RunningProgram program = new RunningProgram(strategy);
Signal.handle(new Signal("TERM"), strategy);
Signal.handle(new Signal("INT"), strategy);
program.start();
}
}
The best thing is that our RunningProgram no longer needs to know what happens if a terminate or interrupt signal is sent to the program. With simply a dependency on the RunStrategy we can know inject a fixed run strategy for tests that we ended up calling a RunNumberOfTimesStrategy. We also promoted the specific RunLikeADaemonStrategy to a full class (not an inner class).
Thanks for getting this far! Please leave a comment if you thought this was useful.
Great example Patrick. I really like your step by step approach here, I often find myself tempted to do too much at one time (which is a good way to introduce bugs).
This is pretty neat. I like how you’ve shown the progressive refactoring that results in cleaner, easier to test, object oriented code.
Thanks for your comments. I intend on trying to demonstrate a few more incremental refactorings since I find the net doesn’t tend to have that many of them. Cheers!
Great refactoring example Patrick! I’d like to add just a little comment about the usage of Mocks as instead of create a real RunStrategy implementation. As it is a very simple interface and the great majority of mock frameworks offer a pretty strict control over the Mock behavior, your example sounds to fit very well to its usage.