この記事は、インテル® デベロッパー・ゾーンに公開されている「The Ultimate Question of Programming, Refactoring, and Everything」の日本語参考訳です。
13. 表形式を使用する
ReactOS* プロジェクト (Windows* 互換のオープンソースのオペレーティング・システム) から抜粋した以下のコードについて考えてみます。このエラーは、次の PVS-Studio 診断によって検出されます。
V560 A part of conditional expression is always true: 10035L. (V560 条件式の一部が常に true です: 10035L。)
void adns__querysend_tcp(adns_query qu, struct timeval now) {
...
if (!(errno == EAGAIN || EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
...
}
説明
上記のコード例は小さく、簡単にエラーを見つけることができます。しかし、実際のコードでは、多くの場合、エラーを見つけるのが非常に困難です。このようなコードを読むとき、無意識のうちに類似する比較はスキップしてしまいます。
これは、条件の書式が悪く、読みづらいため集中しづらく、さらに類似するチェックなので条件にミスがあることはまずないだろうと仮定してしまうことが原因です。
この問題の解決方法の 1 つは、コードを表形式にすることです。
上記のコードでエラーを見つける気になれなかった方のために、このコードでは 1 つのチェックで “errno ==” が不足してます。EWOULDBLOCK はゼロに等しくないため、この条件は常に true になります。
正しいコード
if (!(errno == EAGAIN || errno == EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
推奨事項
最初に、以下は上記のコードを最も単純な「表」形式にしたバージョンです。
if (!(errno == EAGAIN || EWOULDBLOCK ||
errno == EINTR || errno == ENOSPC ||
errno == ENOBUFS || errno == ENOMEM)) {
少し良くなりましたが、まだ十分ではありません。
このレイアウトにはまだ 2 つの問題があります。1 つ目は、まだエラーを見つけづらいこと、2 つ目は、コードの書式を整えるため多くのスペースを追加しなければならないことです。
この問題に対応するため、2 つの改善を行います。1 つ目は、1 行につき 1 つの比較にして、エラーを見つけやすくします。以下に例を示します。
a == 1 && b == 2 && c && d == 3 &&
2 つ目は、演算子 &&、|| などをより合理的な方法で記述します。つまり、右側ではなく左側に配置します。
右側に配置する場合、揃えるためにスペースを追加する必要があります。
x == a && y == bbbbb && z == cccccccccc &&
左側に配置するほうが素早く簡単です。
x == a && y == bbbbb && z == cccccccccc
見慣れないため、最初は違和感があるかもしれませんが、すぐに慣れます。
この 2 つの改善点を、上記のコード例に適用してみましょう。
if (!( errno == EAGAIN
|| EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM)) {
コードの行数は増えましたが、エラーを簡単に見つけられるようになりました。
見た目は奇妙ですが、この形式を使用することを推奨します。私自身、この形式を使用して半年が経ちますが、とても気に入っており、自信をもって皆さんにお勧めします。
私は、コード行が増えることは問題だと考えていません。次のように記述することもあります。
const bool error = errno == EAGAIN
|| errno == EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM;
if (!error) {
コード行が増え、雑然としているのが気になる場合は、関数にすることができます。
static bool IsInterestingError(int errno)
{
return errno == EAGAIN
|| errno == EWOULDBLOCK
|| errno == EINTR
|| errno == ENOSPC
|| errno == ENOBUFS
|| errno == ENOMEM;
}
....
if (!IsInterestingError(errno)) {
皆さんの中には、私が完璧主義者で物事を大げさにしすぎていると思われる方がいるかもしれません。しかし、複雑な表現ではエラーが非常によく起こります。そうでなければ、ここで取り上げたりしません。非常に頻繁に発生し、あらゆるところに存在し、そして見つけるのがとても困難です。
以下は、WinDjView プロジェクトから抜粋した別のコード例です。
inline bool IsValidChar(int c)
{
return c == 0x9 || 0xA || c == 0xD ||
c >= 0x20 && c <= 0xD7FF ||
c >= 0xE000 && c <= 0xFFFD ||
c >= 0x10000 && c <= 0x10FFFF;
}
関数には数行しか含まれませんが、エラーがあります。この関数は常に true を返します。この原因は、結局のところ、書式が悪く、長年コードを保守してきたプログラマーが注意深く読むことを怠ったためです。
このコードを「表」形式に変更して、いくつかの括弧を追加してみます。
inline bool IsValidChar(int c)
{
return
c == 0x9
|| 0xA
|| c == 0xD
|| (c >= 0x20 && c <= 0xD7FF)
|| (c >= 0xE000 && c <= 0xFFFD)
|| (c >= 0x10000 && c <= 0x10FFFF);
}
上記と全く同じ形式にする必要はありません。このセクションの目的は、「無秩序に」記述されたコード中のタイプミスについて注意を喚起することです。「表」形式にすることで、単純なタイプミスを防ぐことができます。このヒントが皆さんの役に立つことを願っています。
注:
正直なところ、「表」形式は害になる場合があります。次の例について考えてみます。
inline
void elxLuminocity(const PixelRGBi& iPixel,
LuminanceCell< PixelRGBi >& oCell)
{
oCell._luminance = 2220*iPixel._red +
7067*iPixel._blue +
0713*iPixel._green;
oCell._pixel = iPixel;
}
これは、eLynx SDK プロジェクトから抜粋したコード例です。プログラマーは、コードの書式を整えるため値 713 の前に 0 を追加しています。残念なことに、このプログラマーは、数値の先頭の 0 はその数値が 8 進数であることを示す、ということを忘れていました。
文字列配列
コードを表形式にする考え方については明確になったかと思います。最後に、もう 1 つの例を紹介します。この例では、表形式は条件だけでなく、さまざまな言語構造で使用できることを示します。
Asterisk* プロジェクトから抜粋した以下のコードについて考えてみます。このエラーは、次の PVS-Studio 診断によって検出されます。
V653 A suspicious string consisting of two parts is used for array initialization. It is possible that a comma is missing. Consider inspecting this literal: “KW_INCLUDES” “KW_JUMP”. (V653 2 つの部分で構成される疑わしい文字列が配列の初期化に使用されました。カンマが不足している可能性があります。リテラル “KW_INCLUDES” “KW_JUMP” を確認してください。)
static char *token_equivs1[] =
{
....
"KW_IF",
"KW_IGNOREPAT",
"KW_INCLUDES"
"KW_JUMP",
"KW_MACRO",
"KW_PATTERN",
....
};
これはタイプミスです。カンマが 1 つ不足しています。その結果、意味の全く異なる 2 つの文字列が 1 つに結合され、次のようになります。
.... "KW_INCLUDESKW_JUMP", ....
このエラーは、プログラマーが表形式を使用していれば防げたでしょう。カンマの不足に気付けたはずです。
static char *token_equivs1[] =
{
....
"KW_IF" ,
"KW_IGNOREPAT" ,
"KW_INCLUDES" ,
"KW_JUMP" ,
"KW_MACRO" ,
"KW_PATTERN" ,
....
};
前述の例と同様に、区切り文字 (この例ではカンマ) を右側に配置すると、多数のスペースを追加しなければならないため面倒です。特に、長い行やフレーズを新たに追加する場合、表全体を再調整する必要があります。
そのため、ここでも次のような表形式を使用することを推奨します。
static char *token_equivs1[] =
{
....
, "KW_IF"
, "KW_IGNOREPAT"
, "KW_INCLUDES"
, "KW_JUMP"
, "KW_MACRO"
, "KW_PATTERN"
....
};
カンマの不足を簡単に見つけられるようになりました。また、コードに多数のスペースを追加する必要もありません。直観的で美しいコードになりました。見慣れない方は違和感があるかもしれませんが、すぐに慣れるでしょう。ぜひ試してみてください。
最後に、私の座右の銘を紹介したいと思います。原則として、美しいコードは通常正しいコードです。
コンパイラーの最適化に関する詳細は、最適化に関する注意事項を参照してください。
