ComposedMethodからあれこれ
http://www.infoq.com/jp/interviews/beck-implementation-patterns-ja
でKentBeckはComposedMethodについてこう言っている。
自分が行おうとしているロジックの意図をコードの読み手へ伝達するのにもっとも効率的な方法は、メソッドをより小さなメソッドから構成することです。大きく長いコードブロックを持たないようにすると同時に、メソッドの中の一行一行の抽象度を一定のレベルにそろえていきます。ですから、ビットシフト演算の次の行で大がかりなオブジェクトの生成のコードが突然出てくることはありません。そのようにコードの抽象度がバラバラだと書き手の意図をうまく伝えることができません。
自分もComposedMethodがパターンとかリファクタリングの中で一番好きで、理由の第1位は簡単だからw
覚えやすい上にSingletonと違って効果は計り知れない。確かに適切な名前が思いつかなかったり、抽出する範囲の選び方とか難しいんだけどね。
で、この部分を聞いていて、ふと数日前に行ったコードレビューを思い出した。
レビュー対象のコードはこんな感じ(if文が追加部分)
void hoge() { /* なんかゴチャゴチャしたながーい読みたくもないコード */ if (huga) { //バグ修正コード(10数行) } /* 前半と同じ */ }
自分の会社はコーディングするといっても、場合によっちゃ追加するのは2・3行という顧客の要望等に応じる糞面白くもないカスタマイズという名の仕事が大半で(この場合はバグ修正だが)、こんな感じのコードはよくある。
でもこのように元のレガシーなコードに今回のような修正を(日付と名前のコメント込みで)、闇雲に付け加えていくと
ますます読む気がなくなるコードになってしまうと思った思慮深い俺はこう指摘した。
「問題ないと思いますが、この追加部分は別メソッドにしません?」
実際の修正は問題なかったので指摘したのはこれくらい。別になんてことないレビューだったんだけどホントにメソッドに抽出してコードは改善したのか?てゆーか改悪されてないか?
KentBeckが言ってる「メソッドを小さなメソッドから構成する」って御言葉に従って追加部分を抽出したはいいけど、
「メソッド1行1行の抽象度を揃える」ってのを破っちまってる。
この場合ベストな道はどれか?
- メソッドhogeの1行1行の抽象度を揃える為に他の部分も別メソッドに抽出する
- 今回の修正でおk
- 元のままGo!
いやケースバイケースなんだろうけどねそりゃ。
そもそもレガシーコードに対してリファクタリングはどのタイミング、どのくらいの範囲で行うべきか?ってのがハッキリしない。
- せめて修正部分はマシなコードにする
- ある程度時間を取って大規模にリファクタリングする
実際は2番目を実行するより0から作り直しってケースが多い気がする。知らんけど。
途中まで読んで積んでるWorking Effectively With Legacy Codeを読めってことか。早くしないと邦訳が出ちゃうしね。