Hatena::ブログ(Diary)

Sacrificed & Exploited

2009-11-25

ポリモーフィズムを使ったリファクタリングの実践例

ポリモーフィズムを使ってないひどいコード設計を見たので、どうリファクタリングするべきだったのかを書いておく。

やばい匂いのするコード

そのシステムでは、複数の銀行とのやりとりを行うため、銀行コードに基づいて処理を分岐していた。そうこんな風に

void 入金(銀行コード, ユーザID, 金額){
    if(銀行コード==0001){
        // A銀行用の処理
        ...
    } else if(銀行コード==0002){
        // B銀行用の処理
        ...
    } else {
       // エラー
    }
}

void 出金(銀行コード, ユーザID, 金額)
    if(銀行コード==0001){
        // A銀行用の処理
         ...
    } else if(銀行コード==0002){
        // B銀行用の処理
         ...
    } else {
       // エラー
    }
}

ここでは2つしか出してないけど、実際はもっと数が多いと思って欲しい。30個くらいはあったかもしれないがよく覚えてない。
考えてみてほしい。銀行コードを判定して処理を切り替えるというコードが何十個もそこかしこに散らばってる様を。そしてそのそれぞれの箇所に修正を入れて、テストをしなければならない手間を。

これは「もともと対応していた銀行が1つしか無かったところに、突貫工事でコードを追加したからこうなった。」と聞いているが、それにしたってもっと賢いやり方があっただろう。技術的負債とはまさにこういった設計の事だ。

リファクタリング

こんな時こそポリモーフィズムの出番だ。ポリモーフィズムをうまく使うと条件分岐を削減できる。
まず銀行用のインターフェースを抽出する。入金とか、出金とか、営業時間内かどうかの判定とか、そういったまさに銀行とのインターフェースとなるメソッドを抽出するのだ。

interface 銀行 {
    void 入金(ユーザID, 金額);
    void 出金(ユーザID, 金額);
    boolean 営業時間内判定(日時);
}

そいで、次に、各銀行ごとにこのインターフェースを実装したクラスを作る。

A銀行 implements 銀行{
    void 入金(ユーザID, 金額){

    }
    void 出金(ユーザID, 金額){

    }
    boolean 営業時間内判定(日時){
        // 9:00から18:00までならtrueを返す
    }
}

B銀行 implements 銀行{
    void 入金(ユーザID, 金額){

    }
    void 出金(ユーザID, 金額){

    }
    boolean 営業時間内判定(日時){
        // 9:00~20:00までならtrueを返す
    }
}

こうすることで、営業時間が違うとか、電文パラメータが違うとか、そういった銀行の違いは個別の銀行クラスの中に閉じ込める事ができる。

あとは、各処理で銀行コードを判定していた処理の代わりに、銀行ごとのオブジェクトを作る際に1回だけ銀行コードを判定して作ってやればいい。そうすれば、この銀行インターフェースを使っている限りはビジネスロジック側で銀行ごとの違いを気にする必要は無くなる。

ビジネスロジック側はこうなる。

銀行オブジェクト bank;

if(銀行コード==0001){
    // A銀行用のインスタンス生成
    bank = new A銀行();
} else if(銀行コード==0002){
    // B銀行用のインスタンス生成
    bank = new B銀行();
} else {
    // エラー
}

if( bank.営業時間内判定()){
    bank.入金(ユーザID, 金額);
} else {
    // 取り扱い時間外メッセージ表示
}

リファクタリングの効果

さて、納品後しばらくたって、銀行をもう1つ追加したいということになったとする。

もし、銀行コードを判定して処理を切り替えるというコードのままだったら、また、

  1. 何十箇所とある銀行判定処理部分を見つけて
  2. その全てに修正をいれて
  3. 元のコードが問題ないか、追加したコードが問題無いか全てテストする

といった事をしないといけない。それも追加したコードがちゃんと動いているかのテストだけでなく、変更前の条件部分がつぶれていないかのテストが必要だ。

リファクタリングで銀行の実装を分離していたとしたら、

  1. 新しく銀行用のクラスを作って
  2. インスタンス生成部分の処理を追加する
  3. 追加した銀行のテストをする

だけだ。もしかしたらインターフェースにメソッドを追加することになるかもしれないけど、それでも修正する範囲、テストする範囲に比べたら少ない。

条件分岐をポリモーフィズムを使う事によって除去でき、また、銀行に関係した処理を1箇所に集中させる事が出来る。もしリファクタリングしていなければ、銀行を増やす度に修正箇所を探して、修正して、テストして、といった膨大な単純作業を毎回ミス無くこなす必要があるのだ。1個でも見逃せばバグになる。

「既存コードを改修するのはリスクが高いのでやらない」とかいう言い訳を聞くけれども、1回だけ集中してやれば後は楽できる方法と、膨大な単純作業を毎回ミス無くこなさなければならない方法、いったいどちらがヒューマンエラーの入り込む確率が高いだろうか?

余談

この方法を学んだのは、リファクタリング―プログラムの体質改善テクニック (Object Technology Series)だったと思う。
だいぶ前に読んだ本なので、確かに書かれていたかどうかは保証できないが、目次にも「料金計算の条件文をポリモーフィズムに置き換える」とあるので、たぶんこれ。

投稿したコメントは管理者が承認するまで公開されません。

スパム対策のためのダミーです。もし見えても何も入力しないでください
ゲスト


画像認証

トラックバック - http://d.hatena.ne.jp/cnaos/20091125/1259136404
リンク元