Do you have any methods in your system that look like the service method in Listing 1? I’ve seen them in many systems. Regardless of how they got there, I often have to change code within them, or add support for new request types. To have enough confidence to make changes, I try to get these methods under the control of unit tests.
How do I even begin to write a test for this code?
Listing 1. A messy method.
public class LibrarySystem {
...
public void service(Request request) {
String command = request.getParameter("command");
if (command.equals("return")) {
String barCode = request.getParameter("barCode");
Date date = new Date();
String branchName = request.getParameter("branch");
Branch branch = findBranch(branchName);
checkIn(barCode, date, branch);
} else if (command.equals("checkout")) {
String barCode = request.getParameter("barCode");
Date date = new Date();
String patronId = request.getParameter("id");
checkOut(patronId, barCode, date);
} else if (command.equals("newBranch")) {
String branchName = request.getParameter("branch");
addBranch(branchName);
} else if (command.equals("addBook")) {
String branchName = request.getParameter("branch");
String author = request.getParameter("author");
String title = request.getParameter("title");
String classification =
request.getParameter("classification");
String year = request.getParameter("year");
Book book = new Book(author, title, classification, year);
String copyNumberParm = request.getParameter("copyNumber");
int copyNumber = 1;
if (copyNumberParm != null) {
copyNumber = Integer.parseInt(copyNumberParm);
}
addNew(branchName, book, copyNumber);
} else if (command.equals("addPatron")) {
String name = request.getParameter("name");
String id = request.getParameter("id");
Patron patron = new Patron(name, id);
add(patron);
}
}
public void add(Patron patron) {
...
}
...
I could talk about how switch statements and if/else statements represent an opportunity to exploit polymorphism. Instead, I’ll take a more test-driven approach: I’ll look to slowly and safely refactor, moving the code into forms that are more easily unit-tested.
I’ll start with the addPatron command functionality. An initial unit test for this chunk of code is shown in Listing 2. The test truly attempts to isolate code as a standalone unit: It verifies that code in the service method calls the LibrarySystem method add(Patron) with a properly-populated Patron argument. Long-term, this is perhaps not the best way to approach the test. Short-term, it’s just fine!
Listing 2.
public class LibrarySystemTest {
@Test
public void addPatron() {
final List<Patron> added = new ArrayList<Patron>();
LibrarySystem system = new LibrarySystem() {
@Override
public void add(Patron patron) {
added.add(patron);
}
};
Request request = new Request();
request.setParameter("command", "addPatron");
request.setParameter("name", "patronName");
request.setParameter("id", "patronId");
system.service(request);
assertEquals(1, added.size());
assertEquals("patronName", added.get(0).getName());
assertEquals("patronId", added.get(0).getId());
}
}
After proving the unit test works, I now can change addCommand code in the service method, with high levels of confidence that I’m not breaking anything. The new else leg now looks like this:
} else if (command.equals("addPatron")) {
AddPatronCommand addPatron = new AddPatronCommand(request, this);
addPatron.execute();
}
The AddPatronCommand class is simply bits of code I moved across to a new class (see Listing 3). Short-term, I passed in the LibrarySystem reference to the constructor of this class. The result is tight coupling: LibrarySystem knows about AddPatronCommand, and AddPatronCommand knows about LibrarySystem. That’s almost always a bad idea, but it’s something I’ll resolve before I check in the code.
Listing 3. AddPatronCommand
public class AddPatronCommand {
private String name;
private String id;
private final LibrarySystem system;
public AddPatronCommand(Request request, LibrarySystem system) {
this.system = system;
name = request.getParameter("name");
id = request.getParameter("id");
}
public void execute() {
Patron patron = new Patron(name, id);
system.add(patron);
}
}
I next rename LibrarySystemTest to AddPatronCommandTest, and then change it to operate directly on AddPatronCommand. See Listing 4 for the modified test. Relevant changes appear in bold.
Listing 4. AddPatronCommandTest
public class AddPatronCommandTest {
@Test
public void execute() {
final List<Patron> added = new ArrayList<Patron>();
LibrarySystem system = new LibrarySystem() {
@Override
public void add(Patron patron) {
added.add(patron);
}
};
Request request = new Request();
request.setParameter("command", "addPatron");
request.setParameter("name", "patronName");
request.setParameter("id", "patronId");
AddPatronCommand command = new AddPatronCommand(request, system);
command.execute();
assertEquals(1, added.size());
assertEquals("patronName", added.get(0).getName());
assertEquals("patronId", added.get(0).getId());
}
}
Renaming the LibrarySystemTest class leaves me with no tests against LibrarySystem. I’ll revisit that problem shortly. First, however, I’ll eliminate the tight coupling between LibrarySystem and AddPatronCommand. That involves moving methods and fields across as appropriate. Obviously, I must move the add(Patron) method. After moving it, I let the compiler tell me what other methods and fields I need to move along with it. It turns out that in the case of add(Patron), I also have to copy across a data access field.
The resulting test and code aren’t much different (see Listings 5 and 6).
Listing 5. AddPatronCommandTest, revised.
public class AddPatronCommandTest {
@Test
public void execute() {
Request request = new Request();
request.setParameter("command", "addPatron");
request.setParameter("name", "patronName");
request.setParameter("id", "patronId");
final List<Patron> added = new ArrayList<Patron>();
AddPatronCommand command = new AddPatronCommand(request) {
@Override
public void add(Patron patron) {
added.add(patron);
}
};
command.execute();
assertEquals(1, added.size());
assertEquals("patronName", added.get(0).getName());
assertEquals("patronId", added.get(0).getId());
}
}
Listing 6. AddPatronCommand, revised.
public class AddPatronCommand implements Command {
private String name;
private String id;
...
public AddPatronCommand(Request request) {
name = request.getParameter("name");
id = request.getParameter("id");
}
public void execute() {
Patron patron = new Patron(name, id);
add(patron);
}
public void add(Patron patron) {
...
}
}
By using this refactoring pattern, I can incrementally put such command classes in place. As I tackle the second command class, I recognize that both commands support the common method execute. I introduce an interface, Command (see Listing 7), and alter each command class to implement this interface.
Listing 7. The Command interface.
public interface Command {
void execute();
}
I am now able to introduce a factory class whose sole job is to return Command objects (see Listing 8).
Listing 8. The Command interface.
public class CommandFactory {
public static Command create(Request request) {
String command = request.getParameter("command");
if (command == null)
return null;
if (command.equals("return"))
return new ReturnCommand(request);
if (command.equals("addPatron"))
return new AddPatronCommand(request);
return null;
}
}
Writing tests for CommandFactory is easy (Listing 9)!
Listing 9. CommandFactoryTest.
public class CommandFactoryTest {
private Request request;
@Before
public void initialize() {
request = new Request();
}
@Test
public void createValidCommands() {
assertCommandType("return", ReturnCommand.class);
assertCommandType("addPatron", AddPatronCommand.class);
}
@Test
public void createWithNoCommandParameter() {
assertNull(CommandFactory.create(request));
}
@Test
public void createWithBadCommandParameter() {
request.setParameter("command", "badCommand");
assertNull(CommandFactory.create(request));
}
private void assertCommandType(String commandArgument,
Class commandClass) {
request.setParameter("command", commandArgument);
assertEquals(commandClass,
CommandFactory.create(request).getClass());
}
}
I now can change code in the service method to take advantage of the command factory (see Listing 10).
Listing 10. Incremental refactoring.
public void service(Request request) {
String command = request.getParameter("command");
if (command.equals("checkout")) {
...
} else if (command.equals("newBranch")) {
...
} else if (command.equals("addBook")) {
...
} else {
libraryCommand = CommandFactory.create(request);
libraryCommand.execute();
}
}
Ultimately, I can refactor all the code in the service method to use the CommandFactory. Here’s what it looks like after moving all chunks of service code out to the command framework:
public void service(Request request) {
Command command = CommandFactory.create(request);
command.execute();
}
Not bad! At this point, I want enough unit test coverage to give me the confidence that the service method uses the factory to execute and execute a command. I don’t need to re-test each command class. Instead, I only need to prove that the execute method gets called for the created command.
I could choose one representative command and verify that it got executed end-to-end. Or I could introduce a “dummy” Command implementation that did nothing but report back that it got executed:
public class NullCommand implements Command {
public static boolean wasExecuted = false;
public void execute() {
wasExecuted = true;
}
}
Along with this change, I alter the definition of the CommandFactory to never return null, instead returning a NullCommand object (see Listing 11).
Listing 11. CommandFactoryTest, revised.
public class CommandFactoryTest {
private Request request;
@Before
public void initialize() {
request = new Request();
}
@Test
public void createValidCommands() {
assertCommandType("return", ReturnCommand.class);
assertCommandType("addPatron", AddPatronCommand.class);
}
@Test
public void createWithNoCommandParameter() {
assertEquals(NullCommand.class,
CommandFactory.create(request).getClass());
}
@Test
public void createWithBadCommandParameter() {
assertCommandType("badComment", NullCommand.class);
}
@Test
public void createDummy() {
assertCommandType("dummy", NullCommand.class);
}
private void assertCommandType(String commandArgument,
Class commandClass) {
request.setParameter("command", commandArgument);
assertEquals(commandClass, CommandFactory.create(request)
.getClass());
}
}
The corresponding CommandFactory implementation is shown in Listing 12.
Listing 12. CommandFactory, revised.
public class CommandFactory {
public static Command create(Request request) {
String command = request.getParameter("command");
if (command != null) {
if (command.equals("return"))
return new ReturnCommand(request);
if (command.equals("addPatron"))
return new AddPatronCommand(request);
}
return new NullCommand();
}
}
The changes to CommandFactory ensure that it cannot possibly return null. My test for service is very simple:
@Test
public void service() {
NullCommand.wasExecuted = false;
LibrarySystem system = new LibrarySystem();
Request request = new Request();
request.setParameter("command", "dummy");
system.service(request);
assertTrue(NullCommand.wasExecuted);
}
Yes, this introduces a NullCommand class into production. It does no harm there, and otherwise it’s a small price to pay for the value provided by better test coverage.
It’s always possible to slowly refactor toward a better design. The improved version of the code demonstrates the Command Pattern, one of the more useful recognized software design patterns. In this case, your need to test proved to be in alignment with good design concepts!
About the Author
Jeff Langr is a veteran software developer with a score and more years
of experience. He’s authored two books and dozens of published articles
on software development, including Agile Java: Crafting Code
With Test-Driven Development (Prentice Hall) in 2005. You can find out more
about Jeff at his site, http://langrsoft.com, or you can contact him directly at jeff@langrsoft.com.
