Saturday, April 16, 2011

The Law of Demeter Makes You Create Mock-Friendly Objects

One of the reasons why I haven't been blogging is that much of the time, I have been busy with teaching -- specifically, as an adjunct in the Master of Software Engineering program at Seattle University (of which I am also an alumnus).
The last two winters, I have taught software design and modeling. Something that I touch on with my students is the Law of Demeter. The LoD states (among a couple of other things) that a software object in an object-oriented program should only talk to its "immediate friends". That rules out long method call chains like this:

customer.getOrders().get(0).getLineItems().get(0).getProduct().getQuantity()

This is bad for at least two obvious reasons. First, the objects in this program are tightly-coupled, and changes to the object model will ripple through the system in annoying ways. Second, if you write this in a Java or C#-like language, rest assured that eventually you will run into a situation where something in this chain is null, and worst of all, the ensuing NullPointerException will not pinpoint exactly what it is. So don't write code like this.

But the reason I'm writing this post is to draw attention to a third disadvantage of long method call chains: They complicate mock object testing.

Let's look at a slightly less pathological example based on the following classes:

public class CustomerController {
  private Customer customer;
  public CustomerController(Customer customer) {
    this.customer = customer;
  }
  public void postOrders() {
    customer.getOrdersList().postOrders();
  }
}
public class Customer {
  private OrdersList ordersList = new OrdersList();
  public OrdersList getOrdersList() {
    return ordersList;
  }
  public void postOrders() {
    ordersList.postOrders();
  }
}
public class OrdersList {
  private List orders = new ArrayList();
  public void postOrders() {
    for (Order order : orders) {
      order.post();
    }
  }
}
public class Order {
  public void post() {
    System.out.println("Posting order");
  }
}

We have a CustomerController that controls (what else?) a Customer and triggers the posting of the customer's Orders. Forget the unlikeliness of this particular design. (In reality, the Customer, OrderList, and Order classes would probably be pure-data classes, and CustomerController would use a service class to post orders.) Let's think about how we would test CustomerController.

Presuming that the Order.post() method ultimately makes external connections, we want to mock the Customer.postOrders() method and simply verify that CustomerController calls it. If we try to write the test this way:

@Test
public void testBadPostOrders() {
  Customer customer = EasyMock.createMock(Customer.class);
  // Next line: NullPointerException!
  customer.getOrdersList().postOrders();
  EasyMock.expectLastCall();
  EasyMock.replay(customer);
  CustomerController controller = new CustomerController(customer);
  controller.postOrders();
  EasyMock.verify(customer);
}

We should have known better than to try to call customer.getOrdersList().postOrders() on an EasyMock mock. To fix this, we could add a mock OrderList, return it from an expected call to customer.getOrderList() on the Customer mock, and create an  orderList.postOrders() expectation. But doing that type of thing will rapidly become annoying. And this is a very simple example.

We can move toward a better solution by noting that in this case, the test itself (which you can think of as client code of the Customer class) is not following the Law of Demeter: It is talking to the OrderList, which is not an "immediate friend". Yet, from the perspective of the test, the Customer class doesn't leave it a lot of choice.

Fortunately, we can refactor the Customer and CustomerController classes to make them more mock-testable. We can add a postOrders() helper method to the Customer class:

public class Customer {
  private OrdersList ordersList = new OrdersList();
  public void postOrders() {
    ordersList.postOrders();
  }
}

And we can change CustomerController to call it:

public class CustomerController {
  private Customer customer;
  public CustomerController(Customer customer) {
    this.customer = customer;
  }
  public void postOrders() {
    customer.postOrders();
  }
}
Then we can easily write a working test that uses the mock framework to verify that the controller does, in fact, post the orders:
@Test
public void testGoodPostOrders() {
  Customer customer = EasyMock.createMock(Customer.class);
  customer.postOrders();
  EasyMock.expectLastCall();
  EasyMock.replay(customer);
  CustomerController controller = new CustomerController(customer);
  controller.postOrders();
  EasyMock.verify(customer);
}

And again, as many others have pointed out, good designs and good tests feed back into one another.

No comments: