バグか仕様か手法の限界か？

　単体テストをしながら、リファクタリングを進めている。単体テストをしてみると、まだまだ期待した動作をしていないことが判明した。再帰テストを行いながら、リファクタリングをして、少しずつモジュールの可読性を向上させ、メンテナンスのしやすさを改善させていく。
　リファクタリングについて思ったことを書いてみる。

[06] リファクタリングの際に注意すべきこと
にある注意を意識する。

何度でも言う。単体テストを充実させよう

リファクタリングする際には単体テストが必須である。今までにバグ、あるいは想定外の動作をした入力データは、貴重なデータです。単体テストをする際には、そのようなデータをどんどん集めていきましょう。不具合を発見してくれた同僚は、あなたの大切な味方です。それらを使いながら、次の視点を参考にしながら、コードの改善をしていきましょう。

・単体テストのテスト入力を分類しておこう

単体テストの入力は、期待される戻り値の種類で予め分類しておくのもいいだろう。特に単体テストの結果を目視で確認しなければならないようなときには効果的だ。

・過去のリビジョンの結果と比較する

diffで再帰テストができる場合はいいが、diffでは簡単にテストできないような処理の場合、劣化を生じていないことを確認するのに目視で確認しなければならない状況が生じる。そのような際には、過去のリビジョンの結果をどのリビジョンの結果かわかるようにして保存しておく。それと現状の版の結果とを比較してみる。いつのまにか劣化を生じた場合がないかを目視で確認してみる。単体テストのテストデータも、追加になっている場合には、過去のアルゴリズムに対して、処理してみる。そうすると、過去の版に対してどれだけ改善が進んだのかを冷静に見ることができる。

・どこで期待と動作が異なるかを確認しよう

たくさんの処理をつっこみすぎた関数では、入力データの挙動について前提としていることがありがちだ。バグか仕様か手法の限界かがはっきりしない関数では、「入力データに対してこういう処理をするとこんな結果になってくれるはず」ということがなんの検証もなしに用いられていることがある（私の経験上）。ある範囲の入力データでは、期待した動作になる正しい関数が、入力データの空間が広がるととたんに「予期せぬ動作」の関数になってしまう。
　この関数では、どのような前提条件でソースコードが書かれているのかを、明示していこう。そうすることで、「バグか、仕様か、手法の限界か」を知る手がかりが得られる。
　そういったことを積み重ねていくと、期待している動作をさせるためには、アルゴリズムをどのように見直す必要があるのかが見えてくるだろう。

リファクタリング

・長すぎるfor文を複数のfor文に分割する

一つのfor文の中の処理があまりに込み入ってしまっている場合、for文の処理を分けた方がわかりやすい。
処理を行うかどうかの条件判定と処理を行う場合の処理の内容がある場合で、処理を行うかどうかの判定部分が自体がかなり複雑になっている場合には、for文を分けて、最初のfor文で処理を行うべき変数のリスト(=std::vector)を作って、次のループでは、その処理を行うというようにfor文を分割したほうが理解がしやすくなる。

変更前のコード

for(int i=0; i<n; i++){
    if(!someCondition1(i)){
        continue;
    }
　　　　 判定までの間に様々な計算が入る。
    if(!someCondition2(i)){
        continue;
    }
    if(!someCondition3(i)){
        continue;
    }
    if(breakCondition(i)){
        break;
    }
    doComplicatedAction(i);
}

変更後のコード

std::vector<int> indexes;

for(int i=0; i<n; i++){
    if(!someCondition1(i)){
        continue;
    }
　　　　 判定までの間に様々な計算が入る。
    if(!someCondition2(i)){
        continue;
    }
    if(!someCondition3(i)){
        continue;
    }
    if(breakCondition(i)){
        break;
    }
    indexes.push_back(i);
}


int m=indexes.size();
for(int j=0; j< m ; j++){
    i = indexes[j]
    doComplicatedAction(i);
}

・for文の中のif文

for文の中にif文があって、if文の中で値を設定している場合、本当に値を設定できているのか、それとも値を設定できていない場合があるのか、そういったアルゴリズムの不明快な部分がリファクタリングを少しずつ行っていくことで見えてくる。if文の中である条件を満たすのか（例：何かがみつかったか、見つからなかったのか）を明快にして、現状十分に考えきられていない部分の仕様をはっきりさせよう。
　
　たいがいの場合、for文の中に書かれているif文がある関数は、仕様の明確化に漏れがおこりやすい（私の経験では）。

ちなみに私の場合は、maxはif文による分岐ではなく、std::max()の関数を使う。if文が消える文だけ可読性が向上する。

２重のfor文の中にあるif文が同一の条件式などということはないか？
そのような場合にはif文をfor文の外に出そう。２重のfor文の中の処理は高くつく。

・ループを途中で打ち切れないか

for文のループは、実は途中で打ち切れることはないか。for文を回す制御変数を逆順にした方が処理が速いということはないか。前から１つ１つたどっていくよりも、乱数を使ってシャッフルした結果を使って探索した方が処理を速く打ち切れるということはないか。こういった工夫の方で得られる効率は、関数呼び出しに変えることで生じるオーバーヘッドに比べ物にならない。
continue;をする場合でも、条件判定を並べる順序しだいで処理時間が短縮することがある。
ロジックを正しく保ったままわかりやすさを向上させること。わかりやすさを改善することで、どう工夫する余地があるのかがわかってくる。

・一度にいろんな処理を関数につっこまない

一度にたくさんのいろんな処理をしている関数は、メンテナンスがとてもしづらい。単体テストはできないし、関数が何をしているのか一言で言えないし、いいことが何もない。単体テストを繰り返し、再帰テストをしながら、リファクタリングしていこう。直列につながるfor文は、それぞれのfor文を含む別の関数に分割したほうがよい場合が多い(私の経験上)。リファクタリングしながら、十分に自明と言えるほど機能が単純明快な関数を取り出していく。関数呼び出しのオーバーヘッドよりも実は別の条件の方が計算時間を浪費していることが多い。

・分解した機能の仕様を明示しよう

おおもとの関数がたくさんのことをしすぎていて、とても単体テストが困難な状況から、関数を抽出に成功してきたら、抽出できた関数についてはその仕様が明確になってきているはずだ。Doxygenで使えるスタイルで関数の仕様を書いていこう。その抽出した関数については、単体テストが可能な状況になってきているはずだ。
　
　

アルゴリズムの見直し

・アルゴリズムの前提条件

あるアルゴリズムが動作するかどうかは、入力データの特性に依存することがある。アルゴリズムが想定している状況を実際の入力データが満たしているかを確認しよう。バグか仕様か手法の限界なのかが判然としない場合には、そのようなチェックをすることが、自分自身の理解を改善するのにつながる。

・問題の定式化は妥当か?

「バグか仕様か手法の限界か？」などというような状況では、解決すべき課題に対して適切な定式化ができていない可能性がある。解決すべき課題に似た既存の枠組みはないか調べてみよう。ほんの少しの違いで既存のよく知られた問題に置き換えることができることがある。そのためにも、アルゴリズムの定石を調べておこう。
　ある種のアルゴリズムの場合だと、最小化問題として定式化されるものが多い。最小化問題を解く手法は、既にたくさんのアプローチがあって、使うばかりになっているライブラリが多い。離散値での最適化の例であるナップザック問題、巡回セールスマン問題、多変数の関数の最小化問題、そういった問題を解く例題を、PythonやMATLABなどのライブラリが充実している処理系で、アルゴリズムの定石に触れてみよう。そうすることで、今かかえている問題に対して別の視点でとらえることができるようになるはずだ。
　問題を解決する際には、ある場合には次の処理、別の場合には別の処理、という思いつきベースのアルゴリズムではなく、数学的な意味を明確にする定式化の方がのぞましいように思う。

・・問題の定式化に成功している場合

　問題の定式化に成功している場合、既存の枠組みを最大限活用することで、効果をあげることができる。よく知られた定石の定式化は、どういうときにうまくいき、どういうときに失敗するのかがわかっている。そのような定石を知るためには、その分野の良書を読むことである。10年、20年以上前にできあがっている古いアルゴリズムであっても、有用なアルゴリズムは相変わらず有用である。20年以上前に有用性が示されているアルゴリズムは、最近のアルゴリズムよりも少ない計算量で実行できるのが多い。どういうときにうまくいき、どういうときに失敗するかがわかっているアルゴリズムでは、失敗をさけるためにどう工夫すればよいのかがわかる。
　入力データへの工夫ができる場合、その定式化が最も効果的になるようにするのがよい。時系列データには、時系列データを扱う際の効果的な定式化、画像データに画像データをを扱う際の効果的な定式化がある。テキストデータマイニングにはテキストデータマイニングに固有の定式化がある。そのような定式化は、データの素性に結びついていることが多く、個別の機械学習のアルゴリズムが変わっても、共通に有用性がある。
　だから、その問題が、既に十分に知られているどのような問題と似ているのか考えよう。問題の定式化に成功すれば、問題は半分解けていることが多い。

・問題を必要以上に難しくしていないか

問題をより一般的な形で解くのはむずかしい。必要十分な範囲で問題を定式化しよう。歩行者を検出する際には、ベンチで横になっている人を検出することは対象外とするのが必要だ。進行方向にある立体物が人か人以外かは、クルマの走行においては、どちらもぶつからないようにするのが重要で、人か人でないかは2の次だ。
抱えている問題に対して、がまんしうる程度の結果をだすことから始めていこう。だれが見てもすばらしい結果をだすのは、その後だ。
えてして必要とされているよりも難しく問題を設定してしまうことがありがちです。
　

・限界を生じる事例は？

限界を生じる事例はどういう場合か、単体テストの入力を増やしながら見つけていこう。データを視覚的に把握してみよう。

・新しい橋が出来る前に古い橋を壊すな

　さんざん手を入れてもうまくいかないからといって、「新しい橋が出来る前に古い橋を壊すな」
「橋を燃やすな原則」

・あえて極端な場合を設定してみる

手法の限界を見極めるには、あえて極端な場合を設定してみるという手がある。例として画像認識をあげて見る。顔検出をするのに、入力画像にノイズを加えていくと、どこかで検出できなくなる。画像にボケを加えていっても、どこかの時点で検出できなくなる。画像のbit深さを減らしていっても検出できなくなる。

アルゴリズムが同じでも、入力データの範囲が変わってくると、それによって破綻することがある。そこそこうまくいったアルゴリズムは、本来の限界を超えて無理な使い方をされてしまうことがある。そのような場合には、いい結果にたどり着けない。
「手法の限界かも」と思ったら、そのような可能性も疑ってほしい。

・バグか仕様か

引継いだコードの元の開発者がいて、その意図を確認できる場合には、その意図を確認しよう。

・新しい方式への準備

新しい方式を考えるためには、関連する情報を仕入れて準備を積み重ねておこう。
最終的に使う方式が１つでも、いろいろ調べておこう。今は使わないけれども、将来使うかもしれないアイディアはいろいろ調べておこう。雑誌を講読して周辺の知識を増やしておこう。必要になったときに、「どの雑誌のどの号にそんなことが書いてあったはず」というメモは役に立ちます。

・依存性を切り離す準備

・その切り替えるべき実装のコードを新たに作ったnamespaceの中に押し込む。
　そうすることで、該当のアルゴリズムを利用している範囲を明らかにできる。
・切り替えるための最上位のインタフェースに対して、互換のインタフェースを持つ別アルゴリズムの実装を作成する。
・動作を比較する。

この記事が、あなたのプロジェクトの単体テストに有用でありますように。

追記：平均を求める関数に空の配列が渡されたときどうすべきか

for文まわりで仕様を十分に決めきっていないことはありがちです。
有名な例は平均値を求める関数に空の配列が与えられた関数の動作です。
合計を求める関数の場合には、0を返すという動作で問題がありませんが、
平均値を求める関数の場合には、問題があります。
ちなみにPythonのnumpy.mean()の場合にはNaNを返し、 RuntimeWarning: invalid value encountered in double_scalars　のwarningを出しています。

追記：

定式化の仕方しだいで問題は解きやすくなる
を書きました。手法の限界に達しているのではないかと思うときの考え方の参考になれば幸いです。