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

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

4. ?: 演算子に注意して、括弧で囲む

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

V502 Perhaps the ‘?:’ operator works in a different way than it was expected. The ‘?:’ operator has a lower priority than the ‘-‘ operator. (V502 ‘?:’ 演算子の動作が期待とは異なる可能性があります。’?:’ 演算子よりも ‘-‘ 演算子のほうが優先されます。)

bool IsVisible(bool ancestorsVisible) const
{
  int16 showLevel = BView::Private(view).ShowLevel();
  return (showLevel - (ancestorsVisible) ? 0 : 1) <= 0;
}

説明

C/C++ 操作の優先順位 (英語) を確認してみましょう。三項演算子 ?: の優先順位は非常に低く、/、+、< などの演算子よりも下です。さらに、- 演算子よりも下です。そのため、プログラムはプログラマーが意図したようには動作しません。

プログラマーは、次の順序で操作が実行されると想定します。

(showLevel - (ancestorsVisible ? 0 : 1) ) <= 0

しかし、実際には次の順序で実行されます。

((showLevel - ancestorsVisible) ? 0 : 1) <= 0

このエラーは、非常に単純なコードでも発生します。これは、?: 演算子がいかに危険であるかを示しています。この演算子は、使用時にミスしやすいため、より複雑な条件での三項演算子は、コードにとって有害です。ミスに気付かない可能性が高いだけでなく、このような式は可読性を損ねます。

?: 演算子には十分に気を付けてください。私は、この演算子に関連した多数のエラー (http://www.viva64.com/en/examples/V502/) を目にしてきました。

正しいコード

return showLevel - (ancestorsVisible ? 0 : 1) <= 0;

推奨事項

以前の記事 (英語) で、三項演算子の問題について述べましたが、その後さらに懸念を深めるようになりました。上記の例は、簡潔な式であっても、ミスを犯しやすいことを示しています。このため、私は以前のヒントを更新することにしました。

?: 演算子を完全に否定するつもりはありません。場合によっては、有益であったり、必要になることもあるでしょう。しかし、過度に使用すべきではありません。また、使用する場合は、次のことを推奨します。

三項演算子は常に括弧で囲みます。

次の式について考えてみます。

A = B ? 10 : 20;

次のように括弧で囲みます。

A = (B ? 10 : 20);

この例では本来、括弧は不要です。

しかし、後でコードをリファクタリングして、次のように X 変数を 10 や 20 に加算する場合、コードを保護します。

A = X + (B ? 10 : 20);

括弧がないと、?: 演算子の優先順位が低いことを忘れてしまう可能性があります。

もちろん、間違って “X+” を括弧内に記述する可能性はありますが、その場合同じエラーになります。括弧は、追加的保護ですが、考慮すべきです。

< 前へ目次次へ >

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