14

動的に割り当てられた配列へのポイントを持つクラスがあるため、コピー コンストラクターと代入演算子関数を作成しました。コピー コンストラクターと代入演算子関数は同じ働きをするので、代入演算子関数からコピー コンストラクターを呼び出しますが、 get "error C2082: redefinition of formal parameter". Visual Studio 2012 を使用しています。

// default constructor
FeatureValue::FeatureValue()
{
    m_value = NULL;
}

// copy constructor 
FeatureValue::FeatureValue(const FeatureValue& other)
{
    m_size = other.m_size;  
    delete[] m_value;
    m_value = new uint8_t[m_size];

    for (int i = 0; i < m_size; i++)
    {
        m_value[i] = other.m_value[i];
    }
}

// assignment operator function
FeatureValue& FeatureValue::operator=(const FeatureValue& other)
{
    FeatureValue(other); // error C2082: redefinition of formal parameter
    return *this;
}
4

4 に答える 4

22

問題のある行は、あなたが思っているものではありません。実際にotherは typeの変数を宣言しますFeatureValue。これは、コンストラクターには名前がなく、直接呼び出すことができないためです。

演算子が virtual と宣言されていない限り、コンストラクターからコピー代入演算子を安全に呼び出すことができます。

FeatureValue::FeatureValue(const FeatureValue& other)
    : m_value(nullptr), m_size(0)
{
    *this = other;
}

// assignment operator function
FeatureValue& FeatureValue::operator=(const FeatureValue& other)
{
    if(this != &other)
    {
        // copy data first. Use std::unique_ptr if possible
        // avoids destroying our data if an exception occurs
        uint8_t* value = new uint8_t[other.m_size];
        int size = other.m_size;  

        for (int i = 0; i < other.m_size; i++)
        {
            value[i] = other.m_value[i];
        }

        // Assign values
        delete[] m_value;
        m_value = value;
        m_size = size;
    }
    return *this;
}

これはうまく機能するか、 Vaughn Catoの回答で提案されているコピーとスワップのイディオムの典型的なガイドラインを使用できます

于 2013-07-05T02:50:47.237 に答える
12

他のメソッドのようにコンストラクターを直接呼び出すことはできません。あなたがしていることは、実際にotherは typeという変数を宣言することですFeatureValue。

代入演算子とコピー コンストラクターの間の重複を回避する良い方法として、copy-and-swap イディオムを見てください。copy -and-swap イディオムとは?

さらによいことに、とのstd::vector代わりにa を使用します。そうすれば、独自のコピー コンストラクターや代入演算子を記述する必要はありません。newdelete

于 2013-07-05T02:50:27.317 に答える
5

短い答え - しないでください。

詳細：

// copy constructor 
FeatureValue::FeatureValue(const FeatureValue& other)
{
    m_size = other.m_size;  
    delete[] m_value;      // m_value NOT INITIALISED - DON'T DELETE HERE!
    m_value = new uint8_t[m_size];

    for (int i = 0; i < m_size; i++)
    {
        m_value[i] = other.m_value[i];
    }
}

// assignment operator function
FeatureValue& FeatureValue::operator=(const FeatureValue& other)
{
    FeatureValue(other); // error C2082: redefinition of formal parameter
    return *this;
}

ノート：

  • コピー コンストラクターが呼び出されると、コピーされるオブジェクトを参照して新しいオブジェクトが構築されますが、既定のコンストラクターはコピー コンストラクターの前に実行されません。これはm_value、コピー コンストラクターが実行を開始したときに不確定な値を持つことを意味します。値を代入することはできますが、そこから読み取ることは未定義の動作であり、delete[]かなり悪いことです (UD よりも悪いことがあるとすれば! ;-))。したがって、そのdelete[]行を省略してください。

次に、operator=コピー コンストラクターから機能を利用しようとすると、最初に既存のデータm_valueが指しているすべてを解放する必要があります。ほとんどの人は次のようにしようとします（これは壊れています）-これがあなたがしようとしていたことだと思います：

FeatureValue& FeatureValue::operator=(const FeatureValue& other)
{
    // WARNING - this code's not exception safe...!
    ~FeatureValue();  // call own destructor
    new (this) FeatureValue(other); // reconstruct object
    return *this;
}

これに関する問題は、FeatureValue の作成が失敗した場合 (たとえばnew、必要なメモリを取得できないため)、FeatureValueオブジェクトが無効な状態のままになることです (たとえばm_value、空間を指している可能性があります)。後でデストラクタが実行されて が実行されると、delete[] m_value未定義の動作が発生します (プログラムはおそらくクラッシュします)。

あなたは本当にこれにもっと体系的にアプローチする必要があります...それを段階的に書き出すか、おそらく保証された非スローswap()メソッドを実装します(簡単に実行できます... and だけstd::swap() m_sizeでm_value、それを ala:

FeatureValue& FeatureValue::operator=(FeatureValue other)
{
    swap(other);
    return *this;
}

これは簡単でクリーンですが、いくつかの小さなパフォーマンス/効率の問題があります。

  • 既存のm_value配列を必要以上に長く保持し、ピーク時のメモリ使用量を増やします... を呼び出すことができますclear()。実際には、問題のデータ構造が膨大な量のデータ (PC アプリの場合は数百メガバイトまたはギガバイト) を保持していない限り、ほとんどの重要なプログラムはこれを気にしません。

  • 既存のm_valueメモリを再利用しようとさえしません-代わりに、常に別newのことを行いotherます（メモリ使用量の削減につながる可能性がありますが、常に価値があるとは限りません）。

最終的に、コンパイラが自動的にコピー コンストラクターを作成するのではなく、個別のコピー コンストラクターが存在する可能性がある理由operator=は、最適に効率的な実装では、一般に、期待どおりに相互に活用できないためです。

于 2013-07-05T03:05:45.637 に答える