Refactoring to Guice: Part 3 of N

In Part 1 and Part 2, I removed several static calls in my pizza program. The services class now benefits from an object-oriented design. Replacing static with non-static methods allows for polymorphism and that helps in testing:
public class PizzaServices {
private final Oven currentOven;
private final Address storeAddress;
private final GeographyServices geographyServices;

@Inject
public PizzaServices(Oven currentOven,
@StoreAddress Address storeAddress,
GeographyServices geographyServices) {
this.currentOven = currentOven;
this.storeAddress = storeAddress;
this.geographyServices = geographyServices;
}

public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
Directions directions = geographyServices.getDirections(
storeAddress, customer.getDeliveryAddress());

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

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

Invoice invoice = Invoice.create(pizzas, directions.getLengthInKm());
return new Order(pizzas, invoice, arrivalTime, customer, directions);
}
}
Polymorphism and static factory methods
Regrettably, the code still has an outstanding static method, the factory method Invoice.create(). The implementation of this method looks up the store address statically:
class Invoice {
public Invoice(List<PizzaSpec> pizzas, int deliveryDistance,
Address storeAddress) {
}
static Invoice create(List<PizzaSpec> pizzas, int deliveryDistance) {
return new Invoice(pizzas, deliveryDistance, PizzaStore.getStoreAddress());
}
}
I made the store address injectable in part 2 so I just need to get that injectable value here. I start by nesting a factory interface inside Invoice.java:
class Invoice {
...
interface Factory {
Invoice create(List<PizzaSpec> pizzas, int deliveryDistance);
}
}
I Implement that interface as an anonymous inner class within my module:
static class PizzaModule extends AbstractModule {
protected void configure() {
...
bind(Invoice.Factory.class).toInstance(new Invoice.Factory() {
@Inject @StoreAddress Provider<Address> storeAddressProvider;
public Invoice create(List<PizzaSpec> pizzas, int deliveryDistance) {
return new Invoice(pizzas, deliveryDistance, storeAddressProvider.get());
}
});
}
}
About the factory's implementation
Everything bound via .toInstance() is injected by Guice when the Injector is created. That way, factories and providers can depend on services provided by other modules. Although constructor-injection is generally preferred, field injection is sufficient in this case.

Within the factory implementation, I bind a Provider<Address> rather than the address directly. This isn't strictly necessary because the address is a constant. But in general, Providers should always be used within factories. This ensures that I always get the correct instance, even if it depends on scope or context. I always use Providers within my implementations of Factory and Provider.

Using the injected factory
To replace the static Invoice.create() method call with the factory, I inject it:
public class PizzaServices {
private final Oven currentOven;
private final Address storeAddress;
private final GeographyServices geographyServices;
private final Invoice.Factory invoiceFactory;

@Inject
public PizzaServices(Oven currentOven,
@StoreAddress Address storeAddress,
GeographyServices geographyServices,
Invoice.Factory invoiceFactory) {
this.currentOven = currentOven;
this.storeAddress = storeAddress;
this.geographyServices = geographyServices;
this.invoiceFactory = invoiceFactory;
}

public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
...
Invoice invoice = invoiceFactory.create(pizzas, directions.getLengthInKm());
return new Order(pizzas, invoice, arrivalTime, customer, directions);
}
}
Now I can test the PizzaServices class without first preparing the static call to PizzaStore.getStoreAddress(). The code makes no static method calls and Guice does the wiring.

Looking forward: Simplifying the factory's implementation
I've had to write a lot of code to implement the Invoice.Factory anonymous inner class. This is the best design, but it's more complex than the static method call. Better code should be easier to write otherwise I usually get lazy and revert to the static factory.

In the next release of Guice, user-defined factory interfaces will be supported. We first annotate the Invoice constructor's injected parameter:
static class Invoice {
public Invoice(List<PizzaSpec> pizzas, int deliveryDistance,
@Inject @StoreAddress Address storeAddress) {
}
...
}
and then we can bind the factory directly to the class it constructs:
static class PizzaModule extends AbstractModule {
protected void configure() {
...
bind(Invoice.Factory.class).toFactoryFor(Invoice.class);
}
}
This is equivalent to our factory inner class, but much more concise. If we change the signature of the Invoice constructor to add or remove injectable paramters, we don't need to update the factory interface.

Guice uses Java's dynamic proxy mechanism to implement the factory interface at runtime. It aggregates the factory method's parameters with injected parameters from the Injector to build the instance at create time.

If you can't wait for Guice 2.0, there's a similar API called AssistedInject that works with Guice 1.1.

Series Conclusion
Now I've removed all the static calls, and the PizzaServices class is perfectly polymorphic and totally testable. In this series, I've demonstrated constant injection, annotations and factory injection.

Replacing static, global code with testable polymorphic code is Guice's core competency. It allows me to decouple interface from implementation. It automatically wires my dependencies for me. If you want to embrace object-oriented design, guicify your app!

1 comment:

Mariano said...

Hi, thanks for this really neat and excellent post!

Just a question, your code ended up like this:

public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
...
return new Order(pizzas, invoice, arrivalTime, customer, directions);
}

Let's suppose that Order is not a value object and it provides lot of behaviours (methods like : prepareFood(), deliverToCustomer(), etc..). In that case we'd probably like to:

a. have different implementations for that Order (in fact 'Order' can be an interface)
b. we would like to inject some more 'services' into it (like you did in 'PizzaServices')

Then (b) looks simple to be done, but we cannot do injection directly due to the fact that the Order implementation requires some arguments in the constructor that cannot be injected by Guice as they are the result of our previous process (pizzas, invoice, arrivalTime, customer and directions). Therefore the only solution that satisfies (a) and (b) is to inject an 'OrderFactory' into PizzaServices and let the code be:

public class PizzaServices {
...
private final Order.Factory orderFactory;


@Inject
public PizzaServices(Oven currentOven,
@StoreAddress Address storeAddress,
GeographyServices geographyServices,
Invoice.Factory invoiceFactory,
Order.Factory orderFactory) {
...
this.orderFactory = orderFactory;
}

public Order createOrder(List<PizzaSpec> pizzas, Customer customer) {
...
return orderFactory.create(pizzas, invoice, arrivalTime, customer, directions);
}
}



and the Guice Module will contain something like this (here I'm supposing that OrderImpl, a particular implementation of Order, requires the StoreAddress to be injected):

static class PizzaModule extends AbstractModule {
protected void configure() {
...
bind(Order.Factory.class).toInstance(new Order.Factory() {
@Inject @StoreAddress Provider<Address> storeAddressProvider;
public Invoice create(List<PizzaSpec> pizzas, Invoice invoice, int arrivalTime, Customer customer, Directions directions) {
return new OrderImpl(pizzas, invoice, arrivalTime, customer, directions, storeAddressProvider.get());
}
});
}
}



So, is this correct? is there another 'more elegant' approach for this problem?

regards and thanks again,
Mariano Ortega