Public Object: Refactoring to Guice: Part 1 of N

Refactoring to Guice: Part 1 of N

In this N-part series, I'm attempting to document some patterns for improving your code with Guice. In each example, I'll start with sample code, explain what I don't like about it, and then show how I clean it up.

The bad code
They're everywhere! Static utility methods that depend on lots of stuff, which must in turn be static:

public class PizzaUtilities {
  private static final int TIME_TO_PREPARE = 6;
  private static final int MAX_DISTANCE = 20;

  public static Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
    Directions directions = Geography.getDirections(
      PizzaStore.getStoreAddress(), customer.getDeliveryAddress());

    if (directions == null || directions.getLengthInKm() > MAX_DISTANCE) {
      throw new InvalidOrderException("Cannot deliver to , " +
          customer.getDeliveryAddress());
    }

    int arrivalTime = TIME_TO_PREPARE
        + Oven.getCurrentOven().schedule(TIME_TO_PREPARE, pizzas)
        + directions.estimateTravelTime();

    Invoice invoice = Invoice.create(pizzas, directions.getLengthInKm());
    return new Order(pizzas, invoice, arrivalTime, customer, directions);
  }
}

What don't I like about it?
Static dependencies on Geography, PizzaStore, Oven and Invoice classes make it impossible to test this method without testing those methods. The consequences of that:

If PizzaStore is slow to initialize, so is my test

I can't test this method until I have the code for Geography, even if that class is being written by another team

I have to remember to call Oven.setCurrentOven() in my test's setUp() method, or the test fails at runtime.

If Invoice.create() depends on an external service such as a payment processing service, my test fails if that service is not available.

A non-static version with the same methods
Create an alternate non-static class that delegates to its static counterpart:

public class PizzaServices {
  public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
    return PizzaUtilities.createOrder(pizzas, customer);
  }
}

Replacing static calls with non-static
Now wherever I have calling code that was calling PizzaUtilities, I can replace it with an injected instance of PizzaServices. For example, this:

class OrderPizzaAction {
  public void order(HttpSession session) {
    Customer customer = session.getCurrentCustomer();
    PizzaUtilities.createOrder(getPizzaSpecs(), customer);
  }
  ...
}

becomes this:

class OrderPizzaAction {
  private final PizzaServices pizzaServices;
  @Inject
  OrderPizzaAction(PizzaServices pizzaServices) {
    this.pizzaServices = pizzaServices;
  }

  public void order(HttpSession session) {
    Customer customer = session.getCurrentCustomer();
    pizzaServices.createOrder(getPizzaSpecs(), customer);
  }
}

Notice that this required me to change the constructor for OrderPizzaAction. If you'd like to postpone changing the constructor until later, you can still use PizzaServices by statically-injecting the PizzaServices instance:

class OrderPizzaAction {
  @Inject public static PizzaServices pizzaServices;

  public void order(HttpSession session) {
    Customer customer = session.getCurrentCustomer();
    pizzaServices.createOrder(getPizzaSpecs(), customer);
  }
}

And then in our module, we prepare the static injection:

class PizzaModule extends AbstractModule {
  protected void configure() {
    requestStaticInjection(OrderPizzaAction.class);
  }
}

Static injection is very helpful as a transitional aide while refactoring from static to non-static code.

One benefit of this work is already available. OrderPizzaAction can now be tested without the Geography etc., by passing in a mock subclass of PizzaServices that overrides createOrder.

Moving logic to the non-static version
Next, let's move the implementations logic from PizzaUtilities to PizzaServices. We'll leave a forwarding method in place in PizzaUtilities so we don't have to update all the callers right away:

public class PizzaUtilities {
  @Inject public static PizzaServices pizzaServices;

  public static Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
    return pizzaServices.createOrder(pizzas, customer);
  }
}

public class PizzaServices {
  private static final int TIME_TO_PREPARE = 6;
  private static final int MAX_DISTANCE = 20;

  public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
    Directions directions = Geography.getDirections(
      PizzaStore.getStoreAddress(), customer.getDeliveryAddress());
    ...
    return new Order(pizzas, invoice, arrivalTime, customer, directions);
  }
}

Notice that we'll used static injection to make our PizzaServices instance available to PizzaUtilities, so we'll need to update the module to prepare that:

class PizzaModule extends AbstractModule {
  protected void configure() {
    requestStaticInjection(OrderPizzaAction.class);
    requestStaticInjection(PizzaUtilities.class);
  }
}

Injecting the non-static version
Now that PizzaUtilities is injected, we can start to inject its dependencies into it. The low-hanging fruit is the Oven.getCurrentOven() singleton. We'll bind that in the module and then we can inject it!

public class PizzaServices {
  private final Oven currentOven;

  @Inject
  public PizzaServices(Oven currentOven) {
    this.currentOven = currentOven;
  }

  public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
    ...
    int arrivalTime = TIME_TO_PREPARE
        + currentOven.schedule(TIME_TO_PREPARE, pizzas)
        + directions.estimateTravelTime();
    ...
  }
}

and then in the Module:

class PizzaModule extends AbstractModule {
  protected void configure() {
    requestStaticInjection(OrderPizzaAction.class);
    requestStaticInjection(PizzaUtilities.class);
    bind(Oven.class).toProvider(new Provider() {
      public Oven get() {
        return Oven.getCurrentOven();
      }
    });
  }
}

This means that whenever an Oven instance is injected, it'll use the old Oven.getCurrentOven() method to get it. Later on, we'll be able to remove that method as well.

Now we can test PizzaServices without a particular Oven instance prepared in advance. In a future post, I'll demonstrate how to remove the remaining static calls.

Part 2

# posted by Jesse Wilson on Tuesday, July 03, 2007

hmm, is the module detected automatically? or is this bootstrapped in main(..)? would be useful to say what jars are needed etc. maybe a zip download? i know much of this is probably in the docs some place but if it's a tutorial..

# posted by Ken Horn on August 22, 2007 1:50 AM