今日の役に立たない一言 - Today’s Trifle! -

古い記事ではさまざまなテーマを書いていますが、2007年以降はプログラミング関連の話がほとんどです。

単一条件単一分岐の原則

grep で検索して、マッチした部分をがしがし書き換えている。と、某社が書いたコードにあたった。ファイルを開いて呆然。あまりにコードが汚くて、リファクタリングの手が止まってしまった。
使っている Meadow はやや長い行でもラップしないように、90桁表示できるように設定してあるんだけど、それで3行とか4行も折り返してあるところがたくさんあったりする。(下のコードいうと、msg = a + b + c + ... と、延々と続いている状態)
変に共通化しようとしていて、mode とかいうインスタンス変数を使って switch で分岐させて、最後に共通の処理を行っている。

    void foo() {
        if (somecondition) {
            mode = SOME;
            param = x;
        }
        else if (anotherCondition) {
            mode = ANOTHER;
            param = y;
        }
            :
        commonMethod();
    }

    viod commonMethod() {
        String msg = null;
        switch (mode) {
        case SOME:
            msg = "some: param=" + param;
            break;
        case ANOTHER:
            msg = "another: param=" + param;
            break;
        case OTHER:
            :
        case ANY:
            :
        }
        printMessage(msg);
    }

見れば分かるけど、commonMethod() は、実は全然共通化の役割を果たしていない。下手な共通化(複雑化?)の典型的な例だ。可読性が落ちまくるだけだし。実際に共通化されているのは printMessage() だけ。
こうした方がよほど読みやすい。

    void foo() {
        String msg = null;
        if (somecondition) {
            msg = "some: param=" + param;
        }
        else if (anotherCondition) {
            msg = "another: param=" + param;
        }
            :
        printMessage(msg);
    }

そもそも、せっかく既に分岐しているものを、mode のような変数でくくって1本のルートに戻して再び分岐するなんて、やる意味がない。プログラムのパスを無駄に増やしてるだけ。
「単一条件単一分岐の原則」とかいう、ソフトウェア原則を勝手に作ってみる。ま、こんなことは今さら言わなくても、既に誰かが言ってるんだろうけど。