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

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

42. 関数名に “empty” を使用しない

WinMerge プロジェクトから抜粋した以下のコードについて考えてみます。このコードにはエラーが含まれています。PVS-Studio アナライザーは、次の診断を出力します。

V530 The return value of function ‘empty’ is required to be utilized. (V530 関数 ‘empty’ の戻り値を利用する必要があります。)

void CDirView::GetItemFileNames(
  int sel, String& strLeft, String& strRight) const
{
  UINT_PTR diffpos = GetItemKey(sel);
  if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
  {
    strLeft.empty();
    strRight.empty();
  }
  ....
}

説明

プログラマーは、strLeft 文字列と strRight 文字列を消去しようとしています。どちらも string 型ですが、これはほかでもない std::wstring です。

文字列を消去するため、プログラマーは empty() 関数を呼び出していますが、これは正しくありません。empty() 関数はオブジェクトを変更せずに、文字列が空かどうかを返します。

正しいコード

この問題を解決するには、empty() 関数を clear() または erase() に置き換えるべきです。WinMerge 開発者が好む erase() を使用すると、次のようになります。

if (diffpos == (UINT_PTR)SPECIAL_ITEM_POS)
{
  strLeft.erase();
  strRight.erase();
}

推奨事項

このコード例では、”empty()” という名前は非常に不適切です。この関数は、異なるライブラリーで 2 つの異なる処理を意味します。

emply() 関数は、一部のライブラリーではオブジェクトを消去し、ほかのライブラリーではオブジェクトが空かどうかを返します。

一般に、”empty” は人によって解釈が異なります。「アクション」と受け取る人もいれば、「情報の照会」と受け取る人もいます。これが、この問題の原因です。

この問題の解決方法は 1 つだけです。クラス名では “empty” を使用しないようにします。

消去関数の名前は、”erase” または “clear” にします。”clear” はあいまいなため、私は “erase” を使用しています。
情報を照会する関数の名前は、”isEmpty” など、別の名前にします。

これは大した問題ではないと考えている方は、http://www.viva64.com/en/examples/v530/ (英語) をご覧ください。これはよくあるエラーパターンです。もちろん、これらのクラスを std::string に変更するのには少し遅すぎますが、少なくともこれ以上被害が拡大しないようにすることはできます。

< 前へ目次次へ >

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