ラベル Legacy Code の投稿を表示しています。 すべての投稿を表示
ラベル Legacy Code の投稿を表示しています。 すべての投稿を表示

2013年7月6日土曜日

モンスターメソッドを解体する

『レガシーコード改善ガイド』の第22章「モンスターメソッドを変更する必要がありますが、テストを書くことができません」を自分なりに整理して、モンスターメソッドへの対処方法を要約してみる。

何をどこまでテストすれば?

まず、どういう種類のテストを書こうとしているか考える。それには、第13章「変更する必要がありますが、どんなテストを書けばよいのかわかりません」が役に立つ。そこでは、〈仕様化テスト〉と〈狙いを定めたテスト〉が紹介されている。〈狙いを定めたテスト〉を書くのが現実的だろう。モンスターメソッドの振る舞いは多様だから、いきなり全体を対象とした〈仕様化テスト〉は書けないことが多そう。

次に、モンスターメソッドを読み込んで、狙いを定める。どこまで狙いを絞り込めるかは、モンスターメソッドの作りと必要な変更によってケースバイケースになる。少なくとも自分にとっては、技芸の世界で、筋道立った説明をできない。

狙いが定まったら、検証方法を考える。この時、検証対象が一意に識別できなかったり (例: 別のブランチから同じ結果が返ってくる) 、テストから見えなかったりしたら、〈検出用変数〉または〈クエリメソッド〉を導入する。この時点では、これ以上の編集は避けた方が無難だと思う。やり過ぎると〈編集して祈る〉のと変わらない。

テストを書く

ここまでで、何のためにどの範囲でどんな検証をしないといけないか、具体的になってくるはず。ここまで来たら、テストを書き始める。書き始めたはいいが、モンスターメソッドを持っているクラスをインスタンス化できなかったり、モンスターメソッドの引数をインスタンス化できなかったりすることが多いと思う。その場合は、別の章を参照して対処する (ここでは深入りしない)。
  • 第09章 このクラスをテストハーネスに入れることができません
  • 第10章 このメソッドをテストハーネスで動かすことができません

変更とメソッド抽出

無事にテストが書けたら、実行してパスすることを確認する。この後、必要な変更を行うか、〈メソッドの抽出〉を行うかは、悩ましいところ。変更が局所的なら先に変更してもいいと思う。逆に変更がモンスターメソッドのあちこちに散らばるようなら、先にメソッドを抽出しておかないと後が苦しくなる。いずれにせよ、ここでは変更については触れずに、メソッドの抽出についてまとめる。

メソッドを抽出するときには、抽出後に残るメソッドの形を想像しておくと、どういう単位で抽出すればいいか方針を決めやすい。残る形には、〈骨組みメソッド〉と〈処理シーケンス〉の2種類がある。

〈骨組みメソッド〉は制御構造だけを持っていて、条件式や各条件下の処理はメソッドに任せてしまう。『実装パターン』だとConditionalに対応する。『デザインパターン』で例えるとFacadeやMediatorのイメージになる。

もう一つの〈処理シーケンス〉は単に順番にメソッドを呼び出していく。『リーダブル・コード』の無関係の下位問題の抽出を行うイメージ。「いちいちメソッドにするほどの問題ではないんじゃ?」と思っても、メソッドにして意図に即した名前を付けておくとリーダビリティが上がるはず (『実装パターン』のExplaining Message)。

どちらが適しているかは、モンスターメソッドの元々の役割によって変わる。大抵の場合、モンスターメソッドは、実装コストが低いところに追加が集中した結果であって、最初からモンスターメソッドとして生まれたわけではない。処理の振り分けが役割だったなら骨組みメソッド、抽象度の高いAPI提供が目的だったなら処理シーケンスを残す方が自然だと思う。

どういう形を残す決めたら、メソッドを抽出する前に、抽出したメソッドのテストを書く。先にテストを書けばレガシーコードは生まれない。アジャイルじゃなくたって、TDDした方がいいと思う。

さらに分割

ここまで来たら、細かい粒度でテストできるようになっている。でも、モンスターメソッドを持っているオブジェクトは、得てしてGod Objectだったりする。抽出したメソッドを〈メソッドオブジェクト〉として取り出して、委譲するように修正することも考えていく。つまり、『レガシーコード改善ガイド』の〈メソッドオブジェクトの取り出し〉、『リファクタリング』のReplace Method with Method Objectを行う。

References


2013年5月18日土曜日

JavaScriptで依存性を排除するためにデフォルトパラメータを導入する

JavaScriptのクラスHogeをテストしたいのだけれど、次のような状況でそもそもインスタンス化が大変な場合にどうするか? という話。
  • Hogeはコンストラクタ内で別のクラスFugaをインスタンス化してプロパティに設定している。
  • Fugaはインスタンス化の際にサードパーティ・ライブラリのAPIを大量に呼び出している。
  • サードパーティ・ライブラリは、開発環境では使えない(例えば、REST APIを提供するサーバがまだ立っていない)。
この場合をコード例で表してみる。サードパーティ・ライブラリが使えないため、Fugaのコンストラクタ呼び出しでエラーが発生し、Hogeをインスタンス化できないとする。
Hoge = function () {
  this.fuga = new Fuga();
  // ...
}
// ...
Fuga = function () {
  this.thirdPartyLibrary = new ThirdPartyLibrary();
  // ...
}
// ...

FugaがThirdPartyLibraryをラップしているから、テスト時にはテストダブルに入れ替えたい。でも、そもそもHogeもFugaもインスタンス化できない。

こんな状況に対応するためのリファクタリングとして、『レガシーコード改善ガイド』は25.14「コンストラクタのパラメータ化」か、デフォルト引数の追加を紹介している。オーバーロードのないJavaScriptでは、コンストラクタをパラメータ化できないので、デフォルト・パラメータを指定する。

デフォルト・パラメータがtrueと見なせる(false、null、0、""、undefinedのいずれでもない)なら、こんな風に短く書ける。||は真偽値ではなくてfugaを返す性質を利用している。
Hoge = function (fuga) {
  this.fuga = fuga || new Fuga();
  // ...
}
// ...

もっとロバストな書き方は次の通り。こちらは、デフォルト・パラメータがfalseと見なされる場合も使える。実用的なのは、デフォルト値が0やfalseの時。
Hoge = function (fuga) {
  if (typeof fuga === 'undefined') {
    fuga = new Fuga();
  }
  this.fuga = fuga;
  // ...
}
// ...

References



JavaScriptにおける検出用変数

JavaScriptで『レガシーコード改善ガイド』の第22章「モンスターメソッドを変更する必要がありますが、テストを書くことができません」で紹介されている「検出用変数の導入」を行ってみる。

『レガシーコード改善ガイド』のサンプルコードはJavaだからインスタンス変数として導入しているけれど、ここでは関数プロパティとして導入する。JavaScriptでは、関数もオブジェクトだからプロパティを持つことができる。

関数プロパティとして導入した方が、使用する場所に近くなるから読みやすくなるし、何かの拍子に誤ってアクセスする可能性が小さくなる。モンスターメソッドはただでさえ長い上に、そいつを持っているオブジェクトもゴッド・オブジェクトだったりするから、どれだけ慎重になってもなり過ぎるということはないはず。

というわけで、godObject.monsterMethodに検出変数isProcessedを導入してみる。これで複雑な条件をかいくぐって目的のブロックが実行されているかどうかを検出できるようになる(この実行が委譲されているなら、モックを使って検出できるけれど、モンスターメソッドが書かれるような状況では期待薄だと思う)。
var godObject = {
    monsterMethod : function (num) {
        // 検出用変数
        this.monsterMethod.isProcessed = false;
    
        if (true) {
            // ... 
            if (false)  {
                // ...
            } else if (true) {
                if (num === 2) {
                    this.monsterMethod.isProcessed = true;
                    // このブロックが実行されているかどうかを検出したい
                }
            } else {
                // ...
            }
        }
    }
}

検出用変数には、godObject.monsterMethod.isProcessedでアクセスできる。実際には、テストコード中でassertTrue(godObject.monsterMethod.isProcessed)のような形でアクセスすることになるだろうけれど、ここでは簡単に確認するためにconsole.logを使う。
godObject.monsterMethod(1);
console.log(godObject.monsterMethod.isProcessed); // > false
godObject.monsterMethod(2);
console.log(godObject.monsterMethod.isProcessed); // > true
godObject.monsterMethod(3);
console.log(godObject.monsterMethod.isProcessed); // > false

この方法は、『JavaScriptパターン』の4.3「関数プロパティによるメモ化パターン」を参考にしている。関数にその関数のキャッシュ変数を持たせられるなら、検出用変数を持たせられるだろう、と。

References