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);
}
}`</pre>
**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:
<li>If `PizzaStore` is slow to initialize, so is my test
</li><li>I can't test this method until I have the code for `Geography`, even if that class is being written by another team
</li><li>I have to remember to call `Oven.setCurrentOven()` in my test's `setUp()` method, or the test fails at runtime.
</li><li>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:
<pre class="prettyprint">`public class Pizza**Services** {
public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
return Pizza**Utilities**._createOrder_(pizzas, customer);
}
}`</pre>
**Replacing static calls with non-static**
Now wherever I have calling code that was calling `Pizza**Utilities**`, I can replace it with an injected instance of `Pizza**Services**`. For example, this:
<pre class="prettyprint">`class OrderPizzaAction {
public void order(HttpSession session) {
Customer customer = session.getCurrentCustomer();
PizzaUtilities._createOrder_(getPizzaSpecs(), customer);
}
...
}`</pre>becomes this:<pre class="prettyprint">`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);
}
}`</pre>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:<pre class="prettyprint">`class OrderPizzaAction {
**@Inject** public **static** PizzaServices pizzaServices;
public void order(HttpSession session) {
Customer customer = session.getCurrentCustomer();
pizzaServices.createOrder(getPizzaSpecs(), customer);
}
}`</pre>And then in our module, we prepare the static injection:`<pre>class PizzaModule extends AbstractModule {
protected void configure() {
requestStaticInjection(OrderPizzaAction.class);
}
}</pre>`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:<pre class="prettyprint">`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);
}
}`</pre>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:
`<pre class="prettyprint">class PizzaModule extends AbstractModule {
protected void configure() {
requestStaticInjection(OrderPizzaAction.class);
requestStaticInjection(PizzaUtilities.class);
}
}</pre>`
**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!<pre class="prettyprint">`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();
...
}
}`</pre>and then in the Module:<pre class="prettyprint">`class PizzaModule extends AbstractModule {
protected void configure() {
requestStaticInjection(OrderPizzaAction.class);
requestStaticInjection(PizzaUtilities.class);
bind(Oven.class).toProvider(new Provider<oven>() {
public Oven get() {
return Oven.getCurrentOven();
}
});
}
}</oven>
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.