プログラミング、リファクタリング、そしてすべてにおける究極の疑問: No. 11

この記事は、インテル® デベロッパー・ゾーンに公開されている「The Ultimate Question of Programming, Refactoring, and Everything」の日本語参考訳です。

11. 1 行にできるだけ多くの操作を詰め込もうとしない

Godot Engine プロジェクトから抜粋した以下のコードについて考えてみます。このエラーは、次の PVS-Studio 診断によって検出されます。

V567 Undefined behavior. The ‘t’ variable is modified while being used twice between sequence points. (V567 未定義の動作です。’t’ 変数は、シーケンスポイント間で 2 回使用される間に変更されます。)

static real_t out(real_t t, real_t b, real_t c, real_t d)
{
  return c * ((t = t / d - 1) * t * t + 1) + b;
}

説明

小さなコードにできるだけ多くのロジックが詰め込まれた複雑な構造を目にしたことがありませんか? これは、コンパイラーの助けにならないだけでなく、ほかのプログラマー (あるいは作成者自身) がコードを読み、理解するのを妨げます。さらに、そのようなコードではミスの可能性が高まります。

一般に、未定義の動作 (英語) が見つかるのは、数行に多くのコードが詰め込まれた領域です。そのようなコードは、多くの場合、1 つのシーケンスポイント (英語) 内で同じ変数を読み書きします。問題をよく理解するためには、「未定義の動作」と「シーケンスポイント」について詳しく述べる必要があります。

未定義の動作とは、コンパイラー実装や最適化オプションに依存する結果を出力する、一部のプログラミング言語の特性です。一部の未定義の動作は (ここで述べるものも含め)、「シーケンスポイント」と密接に関係しています。

シーケンスポイントは、以前の評価に関連する処理はすべて実行済みで、以降の評価に関連する処理はまだ明らかになっていないことが保証されている、コンピューター・プログラムの実行における任意のポイントを定義します。C/C++ プログラミング言語では、次のシーケンスポイントがあります。

演算子 “&&”、”||”、”,”。多重定義されない場合、これらの演算子は左から右へ実行されます。
三項演算子 “?:”。
各完結式の最後 (通常 “;” でマークされる)。
関数呼び出し位置で引数を評価した後。
関数からリターンしたとき。

注: 新しい C++ 標準では、「シーケンスポイント」の概念はなくなりましたが、ここでは「シーケンスポイント」をご存じない方がその概要を簡単に素早く把握できるように上記の説明を使用しています。この説明のほうが新しい説明よりも簡潔で、多くの操作を 1 つにまとめるべきではない理由を理解するのには十分です。

上記のコード例では、前述のシーケンスポイントは使用されていません。また、’=’ 演算子と括弧は、シーケンスポイントではありません。そのため、戻り値の評価で t 変数のどの値が使用されるのか不明です。

つまり、この式が 1 つのシーケンスポイントであり、t 変数がアクセスされる順序は不明です。例えば、部分式 “t * t” は、” t = t / d – 1″ の前に評価されるかもしれませんし、後に評価されるかもしれません。

正しいコード

static real_t out(real_t t, real_t b, real_t c, real_t d)
{
  t = t / d - 1;
  return c * (t * t * t + 1) + b;
}

推奨事項

式全体を 1 行に収めようとすることは、明らかに良いアイデアではありません。読みづらいだけでなく、ミスしやすくなります。

この式を 2 つに分割することで、一度に 2 つの問題を解決できます: コードを分かりやすくし、シーケンスポイントを追加することで未定義の動作を排除できます。

もちろん、上記のコードはほんの一例です。以下に別の例を示します。

*(mem+addr++) = 
   (opcode >= BENCHOPCODES) ? 0x00 : ((addr >> 4)+1) << 4;

前述の例と同様に、このコードのエラーも不合理に複雑なコードによって引き起こされています。プログラマーは 1 つの式内で addr 変数をインクリメントしようとしていますが、式の右辺の addr 変数の値 (元の値とインクリメント後の値のいずれも) が不明なため、未定義の動作になります。

この例に対する最良のソリューションは、前述の例と同様に、不必要に複雑にしないことです。すべての操作を 1 行にまとめる代わりに、操作をいくつかの式に分割します。

*(mem+addr) = (opcode >= BENCHOPCODES) ? 0x00 : ((addr >> 4)+1) << 4; 
addr++;

これらすべてから引き出される単純かつ有益な結論は、数行に一連の操作をまとめないことです。コードをいくつかに分割して、分かりやすくし、エラーの可能性を減らすほうが良いでしょう。

今後は複雑な構造を記述する前に、そのコストとそれに見合うだけの利点が得られるかを慎重に検討してください。

< 前へ目次次へ >

コンパイラーの最適化に関する詳細は、最適化に関する注意事項を参照してください。