コードの理解度を他者が推し量ることの難しさ

11905 ワード

半年ほど前のこと。
若手プログラマーが相談に来てくれました。
そのときのことを思い出したので書いてみます。
タイトルは多少盛っています。 (^^;

相談

既存のコード

既存のコードに次のような関数があります。
読んでもらえれば何となくわかると思うので内容の説明は省略します。

Result MessageReceiver::OnReceived(Message message) {
  switch (message.Type) {
  case TypeA:
    // タイプが A であるメッセージを受信したときの処理
    ...
    break;
  case TypeB:
    // タイプが B であるメッセージを受信したときの処理
    ...
    break;
  ...
  }
}

仕様変更

既存のコードでは TypeA, TypeB, ... は定数 (残念ながらマクロでした) でした。
しかし、仕様変更によって TypeA の値が実行時にならないと確定しなくなるとのことです。
実行時に他のライブラリの関数を呼び、その結果を使って値を確定させます。
この仕様変更にあたって、既存のコードをどのように改造すればよいかという相談です。

条件

提示された条件は主に次の 2 点です:

  • タイプ A の値を確定させるための他のライブラリの関数呼び出しは最初の 1 回だけにしたい。
  • 変更範囲は可能な限り狭くしたい。

回答

この相談に対して、私はいくつかの方法を提案しました:

  1. TypeA だけでなく TypeB も含めて値を確定させて提供する役割を MessageReceiver クラスから分離する。
  2. TypeAMessageReceiver クラスの定数データメンバとし、同クラスのコンストラクタで値を確定させる。
  3. TypeAMessageReceiver::OnReceived メンバ関数のローカル静的定数とし、その初期化中にライブラリの関数を呼び出す。

前述の条件にあるように必要以上に広い範囲のコードに手を入れるのは避けるべきという考え[1]から、可能な限り局所的な改造で仕様変更を実現する案 3 が選ばれました。
実装イメージはこんな感じです:

Result MessageReceiver::OnReceived(Message message) {
  static const int typeA = []() {
    // 外部のライブラリから得た情報を元に、タイプ A の値を確定するコード
    ...
    return xxx;
  }();

  switch (message.Type) {
  case typeA:
    // タイプが A であるメッセージを受信したときの処理
    ...
    break;
  ...
  }
}

その後

相談からしばらく経ち、忘れた頃になってテストチームからバグ指摘が来ました。
指摘の内容は「ログが出すぎている」というものでした。
リポジトリをフェッチして見てみると、リリースに取り込まれたコードはこうなっていました:

Result MessageReceiver::OnReceived(Message message) {
  static const int typeA = []() {
    // 外部のライブラリから得た情報を元に、タイプ A の値を確定するコード
    ...
    return xxx;
  }();
  LOG("TypeA: %d", typeA);

  switch (message.Type) {
  case typeA:
    // タイプが A であるメッセージを受信したときの処理
    ...
    break;
  ...
  }
}

メッセージを受信するたびに、定数 typeA の値をログに出力するようなコードになっていました。
なんで...

お気づきのとおり、ログを出力するのであれば、ローカル静的定数 typeA を初期化しているラムダ式の中に書くべきです。

  static const int typeA = []() {
    // 外部のライブラリから得た情報を元に、タイプ A の値を確定するコード
    ...
    LOG("TypeA: %d", xxx);
    return xxx;
  }();

振り返り

相談に応じる形で関与したので、なぜこうなったのかその後ひとりで考えました。
もちろん、テストが不十分だったということもありますが、次のような可能性を考えてしまいました:

  • このプログラマーは、「最初の 1 回だけ」の条件がどのような原理で実現されているのか理解できていなかったのではないか。
  • レビューした開発者も同様に理解できていなかったか、あるいはきちんと見ていなかったのではないか。

なお、このプロジェクトでは、最低 2 名の開発者が LGTM しないとリリースには出て行かないルール・運用です。

プログラマー本人にも質問してみたのですが、バグを出してしまったことの責任を必要以上に感じているようですっかり「すみません」モードになってしまっていました。

彼は実際には委託先 (つまり他社) の社員で、顔を合わせたことは数回しかなく、年齢もおそらく 10 歳以上離れています。
そんなわけで、私は彼のプログラマーとしてのスキルレベルを正確に把握できていなかったし、彼もわからないことがあっても私に質問しにくい状況があったかもしれません。
(それでも相談に来てくれたのですが)

もちろん、きちんと理解していたけれどもうっかりしていたという可能性もあるでしょう。
ただ、同じコードを前にしたとき、他者がそれをどこまで理解しているのかを推し量るのは難しいことだなと考えるきっかけになりました。
特に、 C++ は人による理解の差が生まれやすい言語ではないかと思います。

普段からこういう話をするとか、理解を近づけるようなことが必要なのだろうかと考えました。
プライドやモチベーションといったものに気をつけながら。

PS

「こういうとき普通はどうするのかな」と思い、先ほど検索してみたところ、次の記事が見つかりました: