2013-07-05 16 views
9

Mam klasę z punktem do dynamicznie przydzielanej tablicy, więc stworzyłem konstruktora kopiowania i funkcję operatora przypisania. Ponieważ konstruktor kopiowania i funkcja operatora przypisania wykonują tę samą pracę, wywołuję konstruktora kopiowania z funkcji operatora przypisania, ale otrzymuję "error C2082: redefinition of formal parameter". Używam programu Visual Studio 2012.Konstruktor wywołania kopiowania z funkcji operatora przypisania

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

Odpowiedz

14

Linia naruszająca wizerunek nie jest taka, jak myślisz. W rzeczywistości deklaruje zmienną other typu FeatureValue. Dzieje się tak dlatego, że konstruktorzy nie mają nazw i nie można ich wywołać bezpośrednio.

Można bezpiecznie wywołać operatora przypisania kopiowania z konstruktora, o ile operator nie jest deklarowany jako wirtualny.

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

Będzie to działa tylko dandys lub użyć typowych wytyczne dotyczące kopiowania & wymiany idiomu sugeruje Vaughn Cato's answer

+0

, dlaczego nie po prostu 'm_value [i] = other.m_value [i]' zamiast używać wartości pośredniej, np. 'value [i] = other.m_value [i]'? – stackunderflow

11

Nie można bezpośrednio wywołać konstruktora, tak jak w przypadku każdej innej metody. To, co robisz, to deklarowanie zmiennej o nazwie other typu FeatureValue.

Spójrz na idiom copy-and-swap dla dobrej drodze, aby uniknąć powielania pomiędzy operatorem przypisania i konstruktora kopię: What is the copy-and-swap idiom?

Nawet lepiej użyć std::vector zamiast new i delete. Wtedy nie musisz pisać własnego konstruktora kopiowania lub operatora przypisania.

+0

It's Bjoink all over again! –

+0

To niesamowite, jak oboje odpowiadaliście na to samo w tym samym czasie. – Borgleader

+0

@Borgleader, co może być jeszcze dziwniejsze, wiem o Vaughnie od ponad 25 lat. On po prostu o mnie nie wiedział;) –

3

Krótka odpowiedź - nie rób tego.

Szczegóły:

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

Uwagi:

  • Kiedy konstruktor kopia jest nazywany, to budowy nowego obiektu w odniesieniu do obiektu kopiowane, ale domyślny konstruktor nie działa, zanim skopiować konstruktora. Oznacza to, że m_value ma nieokreśloną wartość, gdy konstruktor kopiowania zaczyna działać - można do niego przypisać, ale odczyt z niego jest niezdefiniowanym zachowaniem, a do delete[] jest znacznie gorszy (jeśli cokolwiek może być gorsze niż UD! ;-)). Więc po prostu pomiń tę linię delete[].

Następnie, jeśli operator= próbuje wykorzystać funkcjonalność z konstruktora kopii, musi najpierw zwolnić wszystkie istniejące dane m_value wskazuje na lub będzie przeciekał. Większość osób próbuje to zrobić w następujący sposób (który jest uszkodzony) - myślę, że to jest to, co staraliśmy dla:

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

Problem polega na tym, że jeśli powstanie FeatureValue zawiedzie (np bo new puszka nie uzyska wymaganej pamięci), następnie obiekt FeatureValue pozostawia stan nieprawidłowy (np. m_value może być skierowany w przestrzeń). Później, gdy destruktor działa i robi delete[] m_value, masz niezdefiniowane zachowanie (twój program prawdopodobnie zawiesi się).

Naprawdę powinieneś podejść do tego bardziej systematycznie ...albo pisząc go krok po kroku, a może wdrożenie Niegwarantowane rzucających metodę swap() (łatwe do zrobienia ... tylko std::swap()m_size i m_value i używając go ala:

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

To proste i czyste, ale to posiada kilka drobnych problemów wydajność/wydajność:

  • dotrzymał żadnej istniejącej m_value tablicę wokół dłużej niż to konieczne, zwiększenie szczytowego zużycia pamięci ... można nazwać clear() W praktyce większość programów nietrywialne nie obchodzi. o tym, chyba że dat struktura, o której mowa, zawierała ogromne ilości danych (np. setki megabajtów lub gigabajtów dla aplikacji na komputery PC).

  • nawet nie próbuje ponownie wykorzystać istniejący m_value pamięci - zamiast zawsze robi kolejny new dla other (które mogą prowadzić do zmniejszenia zużycia pamięci, ale nie zawsze jest to opłacalne).

Ostatecznie, przyczyny mogą być różne tam konstruktor kopiujący i operator= - zamiast kompilator automatycznie utworzy jeden od drugiego - jest to, że optymalnie wydajne implementacje mogą nie - w ogóle - wykorzystać sobą w sposób miałeś nadzieję.

0

Instrukcja FeatureValue(other); faktycznie wywołuje konstruktora kopii w celu utworzenia nowego obiektu Feature Value, który nie ma nic wspólnego z *this.

Powiązane problemy