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

2018.09.18
その他インテル® DPC++/C++ コンパイラー特集
C/C++エラー検証実装

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

19. あるコンストラクターを別のコンストラクターから適切に呼び出す方法

この問題は、LibreOffice プロジェクトで見つかりました。このエラーは、次の PVS-Studio 診断によって検出されます。

V603 The object was created but it is not being used. If you wish to call constructor, ‘this->Guess::Guess(….)’ should be used. (V603 オブジェクトが作成されましたが、使用されていません。コンストラクターを呼び出すには、’this->Guess::Guess(….)’ を使用します。)

Guess::Guess()
{
  language_str = DEFAULT_LANGUAGE;
  country_str = DEFAULT_COUNTRY;
  encoding_str = DEFAULT_ENCODING;
}

Guess::Guess(const char * guess_str)
{
  Guess();
  ....
}

説明

優れたプログラマーは、重複するコードの記述を嫌がります。それは、素晴らしいことですが、コンストラクターでは、コードを簡潔にしようとして墓穴を掘ることがあります。

コンストラクターは、通常の関数のように呼び出すことができません。”A::A(int x) { A(); }” は、引数なしでコンストラクターを呼び出す代わりに、A 型の名前のない一時オブジェクトを作成します。

これはまさに上記のコード例で起こっていることです。名前のない一時オブジェクト Guess() が作成され、直後に破棄されています。一方、language_str とほかのクラスメンバーは初期化されないままです。

正しいコード

以前は、コンストラクターで重複コードを回避する方法が 3 つありました。

1 つ目は、個別の初期化を実装して、両方のコンストラクターから呼び出します。この方法は明白なので、ここでは例は省略します。

これは、美しく、信頼できる、明確で安全な方法です。しかし、コードをもっと短くしたいと考えるプログラマー向けの方法が次の 2 つです。

これらはかなり危険であり、動作とその影響をよく理解する必要があります。

2 つ目の方法:

Guess::Guess(const char * guess_str)
{
  new (this) Guess();
  ....
}

3 つ目の方法:

Guess::Guess(const char * guess_str)
{
  this->Guess();
  ....
}

2 つ目と 3 つ目では、基本クラスを 2 回初期化しているため危険です。このようなコードは、特定が困難なバグを引き起こす可能性があり、有害無益です。このようなコンストラクター呼び出しが適切な場合とそうでない場合の例を考えてみます。

適切な場合:

class SomeClass
{
  int x, y;
public:
  SomeClass() { new (this) SomeClass(0,0); }
  SomeClass(int xx, int yy) : x(xx), y(yy) {}
};

このコードは、クラスに単純なデータ型のみを含み、ほかのクラスから派生していないため、安全に問題なく動作します。2 つのコンストラクター呼び出しに危険はありません。

適切でない場合:

class Base 
{ 
public: 
 char *ptr; 
 std::vector vect; 
 Base() { ptr = new char[1000]; } 
 ~Base() { delete [] ptr; } 
}; 
 
class Derived : Base 
{ 
  Derived(Foo foo) { } 
  Derived(Bar bar) { 
     new (this) Derived(bar.foo); 
  }
  Derived(Bar bar, int) { 
     this->Derived(bar.foo); 
  }
}

この例では、”new (this) Derived(bar.foo);” と “this->Derived(bar.foo);” でコンストラクターを呼び出しています。

Base オブジェクトは作成済みで、フィールドは初期化されています。コンストラクターの呼び出しにより、ここでも二重初期化が発生します。その結果、新しく割り当てられたメモリーチャンクへのポインターが ptr に書き込まれ、メモリーリークを引き起こします。std::vector 型オブジェクトの二重初期化による影響は、さらに予測が困難です。1 つ明らかなことは、このようなコードは許されません。

これらの問題があると分かっている方法をあえて使用しますか? C++11 を利用できない場合、1 つ目の方法 (初期化関数を作成する) を使用すべきです。明示的なコンストラクター呼び出しが必要になるのは、非常にまれです。

推奨事項

コンストラクターの使用を支援する機能が C++11 で追加されました。

C++ 11 では、コンストラクターから、別のコンストラクター (デリゲート) の呼び出しが許可されます。これにより、最小限の追加コードで、コンストラクターが別のコンストラクターの動作を利用できます。

以下に例を示します。

Guess::Guess(const char * guess_str) : Guess()
{
  ....
}

コンストラクターのデリゲートに関する詳細は、次のリンクを参照してください。

  1. Wikipedia* の C++11 に関するトピック: オブジェクト構築の改良
  2. C++11 FAQ: コンストラクターのデリゲート (英語)
  3. MSDN*: 均一な初期化とコンストラクターのデリゲート
< 前へ 目次 次へ >

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

タイトルとURLをコピーしました