リファクタリングの実例２：第１回

さて、今回は某プロジェクトのWEB系システムで不吉なコードを発見したので問題点を見つけながらリファクタリングを行うことにする。

ひとまず、問題のコードを以下に示す。

public class StupidControl {
    private String シリアルコード;
    private String 製品担当者コード;
    private String 解約月;
    private String 取消月;
    private String 返品月;

    public void execute(int selectType){
        String sql = " SELECT A.商品名, A.数量 ,A.金額 ";
        switch(selectType){
            case 2:
                sql += ", B.解約コード ";
                break;
            case 3:
                sql += ", C.注文取消番号 ";
                break;
            case 4:
                sql += ", D.破損部品番号 ";
                break;
        }

        sql += " FROM";

        switch(selectType){
            case 1:
                sql += " 商品情報 A ";
                break;
            case 2:
                sql += " 商品情報 A, 解約情報 B ";
                break;
            case 3:
                sql += " 商品情報 A, 取消情報 C ";
                break;
            case 4:
                sql += " 商品情報 A, 欠損情報 D ";
                break;
        }

        sql += " WHERE A.製品担当者コード = ? ";

        switch(selectType){
            case 2:
                sql += " AND A.シリアルコード = B.シリアルコード ";
                sql += " AND 解約月 = ? ";
                break;
            case 3:
                sql += " AND A.シリアルコード = C.シリアルコード ";
                sql += " AND 取消月 = ? ";
                break;
            case 4:
                sql += " AND A.シリアルコード = D.シリアルコード ";
                sql += " AND 返品月 = ? ";
                break;
        }

        if(this.シリアルコード != null){
            sql += " AND A.シリアルコード = ? ";
        }

        // Connection の取得
        // PreparedStatementの取得
        // バインド変数の設定
        // SQLの実行
        System.out.println(sql);
    }
}

クライアント

public class Main {
    public static void main(String[] args) {
        StupidControl control = new StupidControl();

        switch(検索タイプ){
            case 1:
                control.execute(1);
                break;
            case 2:
                control.execute(2);
                break;
            case 3:
                control.execute(3);
                break;
            case 4:
                control.execute(4);
                break;
        }
    }
}

元はWEBでリストボックスから検索種類を選択して検索を行うプログラムで、今回は掲載用に改造した。
↓こんな感じ

シリアルコード：
製品担当者コード：
検索タイプ出荷済み解約済み返品済み破損品

同じプログラムで状態によって分岐するというパターンには危険が多く、今回発見したコードもswitch文の多用、クラス分けを行っていない。など、ありがちなパターンになっていた。

ちなみに元のコードでは検索条件が10個ほどあったため、switchの数が半端ではなかった。さらにバインド変数の設定でもSQLの組み立てと同じ条件を使用していたので、コードが2倍になっていた。それらをすべてひとつのメソッドで処理していたため、恐ろしい長さのクラスが出来上がっていた。

次の回から、ちょこちょことリファクタリングを行ってわかりやすいコードに修正しよう。