December 18, 2014
Hot Topics:

Working with Design Patterns: Command

  • April 13, 2007
  • By Jeff Langr
  • Send Email »
  • More Articles »

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) {
      ...
   }
}




Page 1 of 2



Comment and Contribute

 


(Maximum characters: 1200). You have characters left.

 

 


Enterprise Development Update

Don't miss an article. Subscribe to our newsletter below.

Sitemap | Contact Us

Rocket Fuel