レガシーコード改善ガイド 6章 スプラウトメソッド

6章 時間がないのに変更しなければなりません

レガシーコード改善ガイド

6章は、レガシーコードに機能追加や修正をするときに、

  • 既存コードのテストは書かない（書けない）。
  • 既存コードの修正量を最小限にする。
  • 追加するコードは、新しいメソッドや新しいクラスに実装する。

という方針のテクニックです。

  • スプラウトメソッド
  • スプラウトクラス
  • ラップメソッド
  • ラップクラス

この記事では、スプラウトメソッドと、p70のNullを渡す、staticメソッドについて説明します。

スプラウトメソッド

元のメソッドに追加するのは、1行だけ、と決めます。
必然的に、メソッド呼び出しの1行しか書けません。

既存コード p67

public class TransactionGate
{
    public void postEntries(List entries) {
        for (Iterator it = entries.iterator(); it.hasNext(); ) {
            Entry entry = (Entry)it.next();
            entry.postDate();
        }
        transactionBundle.getListManager().add(entries);
    }
}Code language: Java (java)

各エントリに日付を設定して、transactionBundleに格納しているのね

transactionBundleってなんだろ？

深く掘らなくていいと思うわ

新しいエントリかどうかをチェックするコードを加えてほしいって

ありがちな修正 p67

ちゃちゃっと、こんな感じに書くかな〜

public class TransactionGate
{
    public void postEntries(List entries) {
        List entriesToAdd = new LinekdList();
        for (Iterator it = entries.iterator(); it.hasNext(); ) {
            Entry entry = (Entry)it.next();
            if (!transactionBundle.getListManager().hasEntry(entry)) {
                entry.postDate();
                entriesToAdd.add(entry);
            }
        }
        transactionBundle.getListManager().add(entriesToAdd);
    }
}
Code language: Java (java)

んー、すでにゴチャゴチャしてきたわね

スプラウトメソッドバージョン p69

重複エントリを取り除けばいいのよね。重複エントリを取り除く処理を別メソッドに分けたのが、スプラウトメソッドバージョン

public class TransactionGate
{
    public void postEntries(List entries) {
        Liest entriesToAdd = uniqueEntries(entries);
        for (Iterator it = entriesToAdd.iterator(); it.hasNext(); ) {
            Entry entry = (Entry)it.next();
            entry.postDate();
        }
        transactionBundle.getListManager().add(entriesToAdd);
    }

    List uniqueEntries(List entries) {
        List result = new ArrayList();
        for (Iterator it = entries.iterator(); it.hasNext(); ) {
            Entry entry (Entry)it.next();
            if (!transactionBundle.getListManager().hasEntry(entry)) {
                result.add(entry);
            }
        }
        return result;
    }
}
Code language: Java (java)

forループを2回も回していて、処理が遅くならないの？

気にしすぎよ、普通は遅くならないわ。

Nullを渡す

スプラウトメソッドを使いたくても、クラスの依存関係がひどいために、コンストラクタの引数をたくさん擬装しなければインスタンスを生成できないことがあります。代替案の1つは、Nullを渡すを適用することです。

p70

uniqueEntriesメソッドのテストを書かなくていいの？

テスト駆動開発でuniqueEntriesメソッドを書こう、とあるので、本ではテストコードは省略しているのね

こんな感じかな？

@RunWith(JUnit4.class)
public class TransactionGateTest
{
    @Test
    public void uniqueEntries() {
        // foo や barのインスタンスを用意するのが難しい．．．
        TransactionGate transactionGate = new TransactionGate(foo, bar);

        // 省略
        transactionGate.uniqueEntries(entries);
        // 省略
    }
}Code language: Java (java)

fooやbarのインスタンス作るの面倒だな〜、というかできるのかな〜

uniqueEntriesメソッドをテストするために、fooやbarのインスタンスは必要なの？

わからないよ

試しに、TransactionGateのコンストラクタにNullを渡しちゃったら？

マジ？

@RunWith(JUnit4.class)
public class TransactionGateTest
{
    @Test
    public void uniqueEntries() {
        // foo や barのインスタンスを用意するのが難しいので、nullを渡してみた
        TransactionGate transactionGate = new TransactionGate(null, null);

        // 省略
        List actual = transactionGate.uniqueEntries(entries);
        // 省略
    }
}Code language: Java (java)

エラーがなく、テストもグリーンなら、とりあえずはこれでいいと思うわ

公開staticメソッド

これ（Null渡し）が使えない時には、公開staticメソッドを新しく作ることを検討してください。引数として、元のクラスのインスタンス変数を渡す必要があるかもしれませんが、それによりコードの変更が可能になります。この目的のためにstaticメソッドを作ることには違和感を覚えるかもしれませんが、レガシーコードにおいては有用な場合があります。

p70

nullを渡しても、テストが通らないときは？

uniqueEntriesメソッドの中身を、staticメソッドにしちゃうのよ

public class TransactionGate
{
    public void postEntries(List entries) {
        Liest entriesToAdd = uniqueEntries(entries);
        for (Iterator it = entriesToAdd.iterator(); it.hasNext(); ) {
            Entry entry = (Entry)it.next();
            entry.postDate();
        }
        transactionBundle.getListManager().add(entriesToAdd);
    }

    List uniqueEntries(List entries) {
        // インスタンス変数 transactionBundle を渡す
        return uniqueEntriesTestable(entries, transactionBundle);
    }

    // 公開staticメソッド
    static List uniqueEntriesTestable(List entries, TransactionBundle transactionBundle) {
        List result = new ArrayList();
        for (Iterator it = entries.iterator(); it.hasNext(); ) {
            Entry entry (Entry)it.next();
            if (!transactionBundle.getListManager().hasEntry(entry)) {
                result.add(entry);
            }
        }
        return result;
    }
}
Code language: Java (java)

uniqueEntriesTestableメソッドのテストケースを書けそうな気がしてきた。

@RunWith(JUnit4.class)
public class TransactionGateTest
{
    @Test
    public void uniqueEntries() {
        TransactionBundle transactionBundle = new TransactionBundle();

        // TransactionGateのインスタンスを作らずに、
        // staticメソッドを呼んでいる
        List actual = TransactionGate.uniqueEntriesTestable(entries, transactionBundle);
        // 省略
    }
}Code language: Java (java)

TransactionBundleのインスタンス化が難しいときは？

getListManager() を渡すか、フェイクやモックを作るか、とにかく大変よね．．．

staticメソッド群から新しいクラスを作る（１）

いくつかのstaticメソッドを作った後で、それらが同じ変数を共有していることに気づいた場合、それらのstaticメソッドをインスタンスメソッドに持つ新しいクラスを作れる場合がよくあります。

p70

本の方法を改良して、２段階の方法にしたわ
まず、staticメソッドのまま、新しいクラスに移動します。

TransactionGateクラスをこれ以上大きくしたくないわね

TransactionGateクラスから、さっきのstaticメソッドを取り出して、新しいクラスTransactionGateHelperクラスを作ると．．．

public class TransactionGate
{
    public void postEntries(List entries) {
        Liest entriesToAdd = uniqueEntries(entries);
        for (Iterator it = entriesToAdd.iterator(); it.hasNext(); ) {
            Entry entry = (Entry)it.next();
            entry.postDate();
        }
        transactionBundle.getListManager().add(entriesToAdd);
    }

    List uniqueEntries(List entries) {
        // インスタンス変数 transactionBundle を渡す
        return TransactionGateHelper.uniqueEntries(entries, transactionBundle);
    }
}
Code language: Java (java)
public class TransactionGateHelper
{
    static List uniqueEntries(List entries, TransactionBundle transactionBundle) {
        List result = new ArrayList();
        for (Iterator it = entries.iterator(); it.hasNext(); ) {
            Entry entry (Entry)it.next();
            if (!transactionBundle.getListManager().hasEntry(entry)) {
                result.add(entry);
            }
        }
        return result;
    }
}Code language: Java (java)

staticメソッド群から新しいクラスを作る（２）

いくつかのstaticメソッドを作った後で、それらが同じ変数を共有していることに気づいた場合、それらのstaticメソッドをインスタンスメソッドに持つ新しいクラスを作れる場合がよくあります。

２段階目、新しく作ったクラスのstaticメソッドの引数を整理して、インスタンスメソッドにします。

別の機能修正もしてたら、TransactionGateHelperクラスに、staticメソッドが増えました。

どのメソッドも引数でtransactionBundleを渡しているね。

public class TransactionGateHelper
{
    static List uniqueEntries(List entries, TransactionBundle transactionBundle) {
        // 省略
    }

    static List fugaEntries(List entries, TransactionBundle transactionBundle) {
        // 省略
    }

    static List hogeEntries(List entries, TransactionBundle transactionBundle) {
        // 省略
    }
}Code language: Java (java)

TransactionGateHelperをインスタンス化したほうがいいかもね

TransactionGateHelperのコンストラクタでtransactionBundleを渡して、インスタンス変数にして、各メソッドの引数からはtransactionBundleがなくなるんだね。

public class TransactionGateHelper
{
    private TransactionBundle transactionBundle;

    TransactionGateHelper(TransactionBundle transactionBundle) {
        // コンストラクタでtransactionBundleを受け取り、インスタンス変数にする
        this.transactionBundle = transactionBundle;
    }

    // 引数からtransactionBundleがなくなった
    List uniqueEntries(List entries) {
        // 省略
    }

    List fugaEntries(List entries) {
        // 省略
    }

    List hogeEntries(List entries) {
        // 省略
    }
}Code language: Java (java)
public class TransactionGate
{
    public TransactionGate() {
        // どこかで TransactionGateHelperをインスタンス化する。
        transactionGateHelper = new TransactionGateHelper(transactionBundle);
    }

    public void postEntries(List entries) {
        // 省略、同じまま
    }

    List uniqueEntries(List entries) {
        // インスタンスメソッドを呼び、transactionBundleを渡さなくてもよくなった
        return transactionGateHelper.uniqueEntries(entries);
    }
}
Code language: Java (java)
タイトルとURLをコピーしました