Guiceへのリファクタリング

ヒドイコード

こういうのどこにでもあるでしょ！静的なユーティリティメソッドがいろんなものに依存していて、それもまた静的ときたもんだ。

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);
  }
}

なんでこれが気に入らないのか？

Geography, PizzaStore, Oven, Invoiceクラスに静的に依存しているもんで、こいつらもひっくるめてテストをやんなきゃんらない。結果として、

• PizzaStoreの初期化が遅かったりすると、テストも待たされてしまう。

• このメソッドをテストするためには、Geograpyのコードが必要だ。もし別のチームがそれを作っている最中だとしても。
• テストのsetUp()の中で、Oven.setCurrentOven()をセットすることを覚えておかければ。そうしないとテストは失敗してしまう。
• もし、Invoice.create()が外部サービス、例えば支払処理サービスなんかに依存してたりすると、そのサービスがなきゃテストできないじゃん。

同じメソッドの静的でないバージョン

静的でないバージョンを作ってみよう。

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

静的な呼び出しを静的でない呼び出しに置き換える

PizzaUtilitiesを呼び出すコードを、PizzaServicesの注入に置き換えよう。

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

上を以下のように変える。

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);
  }
}

OrderPizzaActionのコンストラクタを変える必要があるよ。 これは後回しにしたいっていうんなら、PizzaServicesを静的に注入することにしてもいいけど。

class OrderPizzaAction {
  @Inject public static PizzaServices pizzaServices;

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

そんでもって、モジュールでは静的注入を指定しとこう。

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

静的注入は、静的から非静的にリファクタリングするあいだ、非常に便利に使えるんだ。

さて、ここまでで既にメリットが生まれた。OrderPizzaActionはGeograpy等々がなくてもテスト可能になった。 createOrderをオーバライドしたPizzaServicesのモックを作って、そいつを渡せばいいのだ。

ロジックを非静的バージョンに変更する

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.