2013-07-05 3 views
9

У меня есть класс с точкой для динамически распределенного массива, поэтому я создал конструктор копирования и функцию оператора присваивания. Поскольку конструктор копирования и оператор-оператор присваивания выполняют ту же работу, я вызываю конструктор копирования из функции оператора присваивания, но получаю "error C2082: redefinition of formal parameter". Я использую Visual Studio 2012.call copy constructor из функции оператора присваивания

// 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; 
} 

ответ

14

Оскорбительная линия - это не то, что вы думаете. Он фактически объявляет переменную other типа FeatureValue. Это потому, что конструкторы не имеют имен и не могут быть вызваны напрямую.

Вы можете безопасно вызывать оператор присваивания копии из конструктора, пока оператор не объявлен виртуальным.

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's answer

+0

, почему не просто 'm_value [i] = other.m_value [i]' вместо использования промежуточного значения, например. 'значение [i] = other.m_value [i]'? – stackunderflow

11

Вы не можете напрямую вызвать конструктор, как и любой другой метод. То, что вы делаете, это объявление переменной типа other типа FeatureValue.

Посмотрите на копирования и обмена идиомы хороший способ, чтобы избежать дублирования между оператором присваивания и конструктор копирования: What is the copy-and-swap idiom?

Еще лучше, если использовать std::vector вместо new и delete. Тогда вам не нужно писать собственный конструктор копий или оператор присваивания.

+0

Это снова Bjoink! –

+0

Свое странное, как вы, ребята, одновременно отвечали на одно и то же. – Borgleader

+0

@ Borgleader, что вы можете найти еще более странным, я знаю о Воне уже более 25 лет. Он просто не знал обо мне;) –

3

Короткий ответ - не делайте этого.

Детали:

// 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 может 't получить память, которую он хочет), то объект FeatureValue остается с недопустимым состоянием (например, m_value может указывать в космос). Позже, когда деструктор работает и делает delete[] m_value, у вас есть неопределенное поведение (ваша программа, вероятно, сбой).

Вы действительно должны подходить к этому более систематически ...либо писать его шаг за шагом, или, возможно, реализации гарантированных нон метания swap() метода (это легко сделать ... просто std::swap()m_size и m_value, и использовать его ала:

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

Это легко и чисто, но это имеет несколько незначительных проблем производительности/эффективности:

  • сохраняя любой существующий m_value массив вокруг дольше, чем это необходимо, увеличение пиковой нагрузки памяти ... можно было бы назвать clear() На практике большинство нетривиальных программ не будет заботиться. об этом, если только рассматриваемая структура содержала огромные объемы данных (например, сотни мегабайт или гигабайт для ПК).

  • даже не пытается использовать уже существующий m_value памяти - вместо того, чтобы всегда делать другой new для other (что может привести к уменьшению использования памяти, но не всегда стоит).

В конечном счете, причины могут быть различными конструктор копирования и operator= - вместо того, компилятор автоматически создает один от другого - это то, что оптимально эффективные реализации не может - вообще - использовать друг друга в пути вы надеялись.

0

Заявление FeatureValue(other); фактически вызывает конструктор копирования для создания нового объекта Featurevalue, который не имеет ничего общего с *this.