2013-07-05 10 views
9

Ich habe eine Klasse mit einem Punkt zu dynamisch zugewiesenen Array, so erstellte ich Kopie Konstruktor und Zuweisung Operatorfunktion. Da Kopierkonstruktor und Zuweisungsoperator die gleiche Arbeit verrichten, rufe ich den Kopierkonstruktor von der Zuweisungsoperatorfunktion auf, bekomme aber "error C2082: redefinition of formal parameter". Ich bin mit Visual Studio 2012.Aufruf Kopie Konstruktor von Zuweisungsoperator Funktion

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

Antwort

14

Die beleidigende Linie ist nicht, was Sie denken, dass es ist. Es deklariert tatsächlich eine Variable other vom Typ FeatureValue. Dies liegt daran, dass Konstruktoren keine Namen haben und nicht direkt aufgerufen werden können.

Sie können den Kopierzuweisungsoperator problemlos vom Konstruktor aus aufrufen, solange der Operator nicht als virtuell deklariert ist.

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

Dieser Wille funktioniert nur Dandy oder Sie die typischen Richtlinien für die Kopie & Swap-Idiom vorgeschlagen in Vaughn Cato's answer

+0

warum nicht einfach 'm_value [i] = other.m_value [i]' anstatt Zwischenwert, z. 'Wert [i] = anderer.m_Wert [i]'? – stackunderflow

11

Sie können nicht direkt einen Konstruktor aufrufen wie jedes andere Verfahren. Was Sie tun, ist eine Variable namens other vom Typ FeatureValue zu deklarieren.

Werfen Sie einen Blick auf die Copy-and-Swap-Idiom für eine gute Möglichkeit, Überschneidungen zwischen dem Zuweisungsoperator und dem Copykonstruktor zu vermeiden: What is the copy-and-swap idiom?

Noch besser ist, verwenden Sie einen std::vector statt new und delete. Dann müssen Sie keinen eigenen Kopierkonstruktor oder Zuweisungsoperator schreiben.

+0

Es ist Bjoink noch einmal! –

+0

Es ist unheimlich, wie ihr beide zur selben Zeit dasselbe geantwortet habt. – Borgleader

+0

@Borgleader Was Sie vielleicht noch seltsamer finden, ist, dass ich seit ungefähr 25 Jahren über Vaughn weiß. Er wusste einfach nicht von mir;) –

3

Kurze Antwort verwenden - es nicht tun.

Details:

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

Hinweise:

  • Wenn die Kopie Konstruktor aufgerufen wird, wird es das neue Objekt mit Verweise auf das Objekt konstruiert kopiert werden, aber das Standard-Konstruktor nicht ausgeführt, bevor die Konstruktor kopieren Das heißt, m_value hat einen unbestimmten Wert, wenn der Kopierkonstruktor gestartet wird - Sie können ihm zuweisen, aber von ihm zu lesen ist undefiniertes Verhalten, und zu delete[] ist es erheblich schlechter (wenn etwas schlechter als UD sein kann! ;-)). Lassen Sie also die delete[] Zeile weg.

Als nächstes, wenn operator= versucht, die Funktionalität von der Copy-Konstruktor zu nutzen, muss es zuerst freigeben alle vorhandenen Daten m_value zeigt auf, oder es wird geleckt werden. Die meisten Menschen versuchen, das zu tun wie folgt (die gebrochen) - Ich denke, das ist, was Sie versuchten:

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

Das Problem dabei ist, dass, wenn die Schaffung von FeatureValue ausfällt (zB wegen new kann bekomme nicht den gewünschten Speicher, dann bleibt das FeatureValue Objekt mit einem ungültigen Status (zB m_value könnte in den Raum zeigen). Später, wenn der Destruktor ausgeführt wird und eine delete[] m_value ausführt, haben Sie ein undefiniertes Verhalten (Ihr Programm wird wahrscheinlich abstürzen).

Sie sollten das wirklich systematischer angehen ...entweder schriftlich es Schritt für Schritt aus, oder vielleicht ein garantiertes nicht werfen swap() Verfahren implementiert (einfach zu tun ... nur std::swap()m_size und m_value, und deren Verwendung ala:

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

, das einfach und sauber, aber es ein paar kleinere Leistung/Effizienz Probleme hat:

  • alle vorhandenen m_value Anordnung um länger als nötig zu halten, maximale Speichernutzung zu erhöhen ... könnten Sie clear() nennen In der Praxis sind die meisten nicht-triviale Programme würden nicht. darüber, wenn die dat Eine fragliche Struktur enthielt große Datenmengen (z. Hunderte von Megabyte oder Gigabyte für eine PC-App).

  • nicht einmal versucht, die bestehenden m_value Speicher wieder zu verwenden - statt immer ein anderen new für other tun (das zu geringerem Speicherverbrauch führen kann, ist aber nicht immer sinnvoll).

Letztlich gibt die Gründe können verschiedener Copykonstruktor und operator= sein - anstatt die Compiler automatisch einen von den anderen schaffen - ist, dass möglichst effiziente Implementierungen nicht - im Allgemeinen - seitig nutzen in der Art und Weise du hättest gehofft.

0

Die Anweisung FeatureValue(other); ruft tatsächlich den Kopierkonstruktor auf, um ein neues Featurevalue-Objekt zu erstellen, das nichts mit *this zu tun hat.

Verwandte Themen