デバッグ用のコードがメンテナンスを妨げてないか


コードをメンテナンスしていると、デバッグ用のコードが不必要に大きくなって、コードの書き換えを妨げていることがある。
そのことについて、個人の経験に基づくことを述べる。
(これらは、後述のようにテスト駆動開発ができていれば、防げることです。テスト駆動開発に移行できなかった時点での私の教訓です。)

デバッグ用のコードのタイプ

デバッグ用のコードにもいろんなタイプがある。
・assert()の実行
・実行したコードの位置を示すための表示
・値の表示
・画像の表示
・理解を助けるためのデータの加工
・デバッグ用と称しつつ、いつの間にか、それなしでは動作しなくなってしまったコード

これが、マクロ定数と組み合わせたときには、経験したくない状況が生まれる。

#define DO_DEBUG
#define DEBUG_LOG
#define DEBUG_SOMETHING
#define TEST

複数の開発者が思い思いのデバッグ用のマクロ定数があって、

#ifdef 
#ifndef

などが山ほどころがっている状況です。
そうすると、マクロ定数の組み合わせによっては、ソースコードがビルドできない状況に陥ってしまいます。

デバッグ用の変数の状況

デバッグ用の変数にもいろんな状況がある。
・デバッグモード(あるいはその他のデバッグ用の定数をonにした状況)のon/offにかかわらず、常に意味のある変数を保持している場合。
・デバッグモードがonのときだけ、正しい値を持っていて、それ以外ではでたらめの値になっている場合
・デバッグモードがonのときだけ、変数が定義されていて、それ以外のときは変数自体が存在しない場合
・デバッグモードのコードがほったらかしになってしまっていて、もはや正しい意味を返していない場合

そのような状況になってくると、デバッグ用のコードが、本来のコードの理解やコードの改良を妨げる状況になってきてしまう。

望ましい方向

  • テストをデバッグ用のコードを含めた実行によって確認するのではなく、単体テストによってそれぞれのモジュールの動作を検証すること

    • 関数内部に#if DEBUB #endif の中に書く、あるいはif(debug){}の中に書くのではなくて
    • GoogleTestなどのテスト用の枠組みの中でvoid test_someFunction();というテストを実装するように書く。
    • この結果、実装用のコードの中にassert()が残る理由がなくなる。
  • 動作時の途中経過の出力の必要性を減らす。

    • その関数に対して単体テストが完了していて、期待通りの動作をしていることが確認できるならば、途中経過を出力するのは無駄だから削除しよう。
  • どうしても途中結果を残す必要があるならば、ログ出力の仕組みをオブジェクト指向での設計で実装しよう。

    • その場合に、debugという文字列をメソッド名につけないこと。
  • 自作ライブラリのユーザーが間違った入力をする可能性があるときには、assert()を残す。
     cv::Matのような込み入ったデータ構造を扱っていると、そのデータ構造を使っている自作ライブラリの想定している入出力をわかりやすくする目的で使う。grayscale画像を入力として想定しているときにカラー画像を入力されてトラブルを生じるのを予防すること。
    void someFunc(const cv::Mat &mat){
    assert(mat.channels()==1);
    .....
    }
    などと書くと、入力データの想定している範囲がどうなっているのかを明示的に示すことができる。

>デバッグ用の変数の領域確保、領域の解放にともなうエラーで、エラーをつぶしていると、本当の目的のコードの改良に割かれる時間が減ってしまう。そういう本末転倒な状況を起こしてしまいやすい。
引継いだ時点のそのようなコードは、もう対処済みです。

さあ、早く単体テストに移行し、自作ライブラリにある不要なデバッグ用の変数を削除し、楽しい開発に移行しよう。

追記:
 その後、アルゴリズムの検証を必要とする部分を、ツールとして単独のコマンドラインアプリケーションとして再実装しました。そうすると、そのアルゴリズムをテストする部分を、コマンドラインアプリケーションに与える入力ファイルを変更することで、いくつもの条件でテストできるようにできました。そのテストに関する部分で途中経過をファイルに保存するようにしても、同じアルゴリズムを用いているもともとのアプリケーションのコードをいじることなしに実現できるようになりました。
 コマンドラインアプリケーションを生成するときにリンクするモジュールが、もともとのアプリケーションと同じくらい多くなっても、とりあえず気にしなくてなんとかなります。
 コマンドラインアプリケーションとして作っておけば、バッチ処理やスクリプト言語から起動させることで、多数の条件に対して処理させることができます。pythonからMatplotlibを用いてグラフを書くこともできます。
 デバッグ用のコードをメインのアプリケーションの中に増やしすぎないことが、プログラムの開発を楽にしてくれます。

CppUnitの導入で得られた恩恵
を書きました。デバッグ用のコードが元々の実装のコードの見通しやメンテナンス性を悪くしている場合には参考にしてください。

追記:

 デバッグ作業が泥沼になりがちなコードは、そもそも関数の仕様が十分に明確になっていないことがことが多い。仕様を十分に明らかにする。間違った解釈ができないまでの明快な仕様を作ることを目指してヘッダファイルを記述すること。
 そのようなヘッダファイルを作成したら、そのヘッダファイルをコミットする。
 次に、そのヘッダファイルに定義された関数のテストを記述する。そのテストコードが書けないならば、実装すべき関数について理解していなことを意味する(と思ってよいのだろう)。テストコードは、どんなに自明なはずのテストでもよいから書き出すことに意味がある。自動化されたテストを作ることで、少なくとも、その部分については保証された実装が出来始める。
 そのようなテスト駆動開発を始める前の自分は、何をどうやって実装・開発すべきかわからないまま、泥沼の何をどうしたらよいのか不明なコードを試行錯誤することに時間を浪費してしまっていた。しかも、他人が書いたコードで役割が不明確な(つまり仕様が不明確な)関数をいじりながら、何をどうしたらよいのかに途方にくれていた。
 その後、別のアプローチをする中で、十分に明確な仕様のアプローチをするようになった。その作業の中では、関数の仕様が明確にできたので、単体テストを開発して、バグが隠れていたことをあぶりだすことができた。
 仕様が明確になっているので、テストを作成することも、見つかったバグに対する修正をすることも、分業が可能になる。実装する本来の関数が訳のわからない記述のコードで分かりにくくなることを防止することができる。
 デバッグのコードがメンテナンスを妨げているのではないかと感じたら、ここに書いた内容を考慮してみてください。

その後は、テスト駆動開発を意識するようになったので、「デバッグ用のコードがメンテナンスを妨げてないか」
という状況はほぼ、発生しにくくなりました。

追記(その2)

この記事を書いた頃と大幅に開発スタイルの違う開発環境で作業した経験に基づいて書き足す。

CircleCI はぜったい使うべき。

理由
- 継続的にテストが自動化される環境にないと、ソースコードは簡単に劣化してしまう。
- 開発者自体がテストを実行するのを繰り返すのは、開発者の開発時間を奪ってしまう。
- いちど自動化テストを書いてしまえば、ソースコードが劣化する危険を予防できる。
- C++, Python など複数の言語で自動化テストの枠組みが用意されている。

自動化テストでテストしてしまえば、実運用のコードのなかに残すテスト用のコードは少なくできる。

  • 問題があったときにassert を生じるコードを実行コードに残さなくて済む。
  • assert で死んでいいコードと、死んではいけないコードがある。
  • モジュールの単体テスト(あるいはクラスの単体テスト)は、極力 自動化テストに移行させてしまう。
  • そうすれば、実運用のコードが汚くなることを減らせる。

logger を使ってprintデバッグをなくそう

  • printデバッグは、ソースコードを汚くしてしまう。
  • printデバッグは、実行時の標準出力を汚くしてしまう。
  • printデバッグは、オーバーヘッドの高いprintf()を呼び出してしまうのでプログラムの動作を遅くしてしまう。
  • printデバッグは, #ifdef #endif を招き寄せてしまう。
  • loggerを使って、本当に見る必要があるときだけ、その出力を見ることができるようにする。そうすることで、通常時の標準出力がきれいになる。
  • 出力先がerror, warningなどのレベルで区別できるので、気にしなければならない重要度別に区別された情報を受け取ることができる。