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