2013-07-05 18 views
9

Ho una classe con un punto ad array allocato dinamicamente, quindi ho creato il costruttore di copia e la funzione di operatore di assegnazione. Poiché la funzione di costruzione di copia e assegnazione operatore esegue lo stesso lavoro, richiamo il costruttore di copie dalla funzione dell'operatore di assegnazione ma ottengo "error C2082: redefinition of formal parameter". Sto usando Visual Studio 2012.call copy constructor dall'assegnazione funzione di operatore

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

risposta

14

La riga incriminata non è quello che si pensa. In realtà dichiara una variabile other di tipo FeatureValue. Questo perché i costruttori non hanno nomi e non possono essere chiamati direttamente.

È possibile richiamare in sicurezza l'operatore di assegnazione copia dal costruttore purché l'operatore non sia dichiarato virtuale.

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

Questa volontà funziona solo Dandy oppure è possibile utilizzare le linee guida tipiche per lo swap linguaggio copia & suggerito nel Vaughn Cato's answer

+0

perché non solo "m_valore [i] = altro.m_valore [i]" invece di utilizzare il valore intermedio, ad es. 'valore [i] = altro.m_valore [i]'? – stackunderflow

11

Non è possibile chiamare direttamente un costruttore come qualsiasi altro metodo. Quello che stai facendo in realtà sta dichiarando una variabile chiamata other di tipo FeatureValue.

Date un'occhiata al linguaggio di copia e swap per un buon modo per evitare doppioni tra l'operatore di assegnazione e il costruttore di copia: What is the copy-and-swap idiom?

Ancora meglio, utilizzare un std::vector invece di new e delete. Quindi non è necessario scrivere il proprio costruttore di copia o l'operatore di assegnazione.

+0

È di nuovo Bjoink! –

+0

È inquietante come voi ragazzi avete risposto allo stesso tempo allo stesso tempo. – Borgleader

+0

@Borgleader quello che potresti trovare ancora più strano è che conosco Vaughn da oltre 25 anni. Semplicemente non sapeva di me;) –

3

Breve risposta - non farlo.

Dettagli:

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

Note:

  • Quando il costruttore di copia viene chiamato, è la costruzione del nuovo oggetto con riferimento all'oggetto la copia, ma il costruttore di default non eseguito prima della copia costruttore. Ciò significa che m_value ha un valore indeterminato quando il costruttore di copie si avvia, è possibile assegnarlo, ma leggere da esso è un comportamento indefinito e a delete[] è notevolmente peggiore (se qualcosa può essere peggiore di UD! ;-)). Quindi, lascia fuori quella linea delete[].

Poi, se operator= cerca di sfruttare la funzionalità dal costruttore di copia, deve rilasciare prima i dati esistenti m_value sta indicando o sarà trapelato. La maggior parte delle persone cercano di farlo nel modo seguente (che è rotto) - Credo che questo sia quello che stavi cercando per:

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

Il problema di questo è che se la creazione di FeatureValue fallisce (ad esempio perché new può 't ottenere la memoria desiderata), quindi l'oggetto FeatureValue viene lasciato con uno stato non valido (ad esempio m_value potrebbe essere puntato nello spazio). Successivamente, quando il distruttore viene eseguito e fa un delete[] m_value, si ha un comportamento indefinito (il programma si bloccherà probabilmente).

Si dovrebbe davvero affrontare questo più sistematicamente ...o scrivendo fuori, passo dopo passo, o forse l'attuazione di un metodo garantito non gettare swap() (facile da fare ... basta std::swap()m_size e m_value, e usarlo ala:

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

che è facile e pulito, ma ha un paio di piccoli problemi di prestazioni/efficienza:

  • mantenendo qualsiasi m_value array esistente più a lungo del necessario, aumentare l'utilizzo della memoria di picco ... si potrebbe chiamare clear() In pratica, la maggior parte dei programmi non banali non mi importerebbe. su questo a meno che il dat una struttura in questione conteneva enormi quantità di dati (ad es. centinaia di megabyte o gigabyte per un'app per PC).

  • nemmeno cercando di riutilizzare la memoria esistente m_value - invece sempre fare un altro new per other (che può portare a ridurre l'utilizzo della memoria, ma non è sempre la pena).

In definitiva, le ragioni non ci può essere distinto costruttore di copia e operator= - piuttosto che avere il compilatore crea automaticamente uno dall'altro - è che le implementazioni ottimale efficienti possono non - in generale - leva reciprocamente in modo speravi.

0

La dichiarazione FeatureValue(other); invoca effettivamente il costruttore di copie per creare un nuovo oggetto Featurevalue, che non ha nulla a che fare con *this.

Problemi correlati