引継いだソースコードには深すぎるif文・for文の入れ子があった。ようやくそれを解消できる見通しがついてきたので、そこから得た教訓を書いてみる。
まだ手をつけるな。テスト環境が先だ
プログラムの断片を書き変えているときには、それによって、バグを生み出している可能性は高くなる。
深すぎる入れ子のif文は、長すぎる関数のソースコードに見られがちの症状だ。可読性を損なっていることは明白なので、書き直したいという衝動に駆られる。しかし、書き換えによって動作を変えてしまっていないことをチェックできるようにテスト環境を用意してからにしよう。
テスト用のデータセットを準備する
そのルーチンをテストするのに十分なテストセットを用意する。
SVNなどをバージョン管理を導入し、ソースコードを管理しておく。
テスト環境ができても、すぐには手をつけない理由
1. ぶら下がりのelseの罠
深すぎる入れ子のif文は、if()else if() elseの対応関係を間違えやすい罠がある。
2. elseの記述がない罠
複雑なif文の入れ子を生じると、elseのときの動作がどうなるのかがわからなくなって、そのときにこのコードはどういう結果になるのかがわからなくなる。
3. 副作用の罠
いっけんif文で判定する順序を入れ替えてもよさそうなコードでも、関数が副作用を持つ場合だと、順序を入れ替えると挙動が変わってしまう。
4. 0回のforループの罠
for文は０回のループの可能性がある。
for文の中で値を設定していると、０回ループのfor文だと、そこでは値を設定されていないということを見落としがちである。
5. breakで抜けたときと最後まで到達したときの区別を無自覚なときにはまる罠
for文の中にbreakがあるときには、forループを抜けたときの条件には、breakでぬけたときとbreakをせずに抜けた場合とがあって、それによってその後の処理の意味合いが変わってくる。breakしたときの制御変数が重要な意味をもつ場合がある。
6. continue文だけしか通らず、実質的な処理がされない可能性を無自覚なときにはまる罠
for文の中にcontinue文があるときには、for文の中で実質的な処理がされずに、for文を抜けてくる場合と、意味がある処理をされてくる場合とがある。その場合のどちらかということ。
7. 検索処理であることを無自覚である場合の罠
for文とbreakの組み合わせは、実は検索をしていて、条件を満たす最初の要素を見つけ出して処理している場合がある。その場合には、検索処理までをfor文で書き、検索に成功した場合の処理と検索に失敗した場合の処理とをfor文の外に書いたほうがわかりやすい。
8. 抽出すべき関数が隠れている罠
深すぎるif文for文の入れ子は、「関数の抽出」をした方がよい場合もある。
自分がいま相手にしているコードが、どのようなコードか十分わかっていない段階で、安易な書き換えは、バグを導入する危険があまりにも高い。
それらの理由から、あまりに階層が深く、何を行っているのかが理解できていないコードには慎重に対処したい。
準備しておこう
・内部で呼び出している関数の副作用について用心するためにも、引数にはconst属性を付ける。入力で値を渡しているのにもかかわらずconstをつけられない変数がある場合には、その理由をその関数のドキュメンテーションコメントに書いておく。
・どういう条件でその深いif文やfor文の制御を抜けるのかを読み解こう。
数百行もあって、入れ子が6以上にもあるようなコードの場合、どのような条件で制御を抜けるのかがまちまちである。
・聞き取り調査をしよう
そのコードを書いた人が身近にいる場合、聞き取り調査をしよう。あなたが1時間ソースコードを読んでも気づき得ないことを、コードを書いた人は知っている。
そのコードが実行させるときの前提条件がどうかによって、「正しいプログラム」かどうかは変わってくる。
コーディングスタイル
・if文の後が一文であっても、ifには必ず{}を用いる。
・for(int i=0 ; i< iend; i++){}のようにループの変数iはforの中に書く。
そうできない場合には、iを有効な値としてfor文のあとで利用していることを示している。
(例：配列に対して検索をしている場合、条件を満たしたiの値を利用していることがある。)
・他にも変数のスコープを可能な限り狭くしておく。
{}である範囲を囲ってみる。
そのような準備作業を行って入れ子を減らす準備をしておく
入れ子を１つ減らすごとに回帰テストをする
テストが実行できる状況になって、少しずつ書き換えを行っていく。
・elseは何に対応しているのかを注意深く確かめておく。
・省略されているelseはどういう条件なのか
・入れ子の階層を１つ浅くしたたびに、動作を変えていないことを回帰テストでテストする。
(この回帰テストをして動作を変えていないことを検証するためにも、十分な数のテストデータがそろわないうちは、ソースコードの書き換えを先送りした。)
以下のページは、書き換えのヒントになるでしょう。
追記：深すぎるif文for文の入れ子を書き換えるとどうなったか。
・ある種の判定条件の関数が切り出された。
・for文を用いた検索関数が切り出され、検索で見つかったとき、見つからなかったときの動作の記述に変わった。
・ある処理を行う場合の前提条件のチェックがさまざまな項目でチェックされて、
それらの前提条件を満たしたときだけ処理を行うことが明確になった。
・条件を満たしたときの動作が、for文を含むものでも意味のまとまりとして抽出されるようになった。
・その中で使われているロジックが明確になったので、現状のロジックの前提や課題が見えやすくなりました。
追記：
このようなコードの場合、関数の外部仕様が十分自明という状況になっていない懸念があります。このようなコードで、Doxygenでドキュメンテーションコメントに基づいてドキュメントを生成しても品質は確保できません。外部に委託した開発では、このようなレベルでドキュメント作成のコストを減らすために、Doxygenを使ってはならないでしょう。