5

W języku C++, gdy klasy zawierają dynamicznie przydzielane dane, zwykle rozsądnie jest zdefiniować jawnie konstruktora kopii, operatora = i destruktora. Ale działanie tych specjalnych metod nakłada się. Bardziej konkretnie operator = zwykle najpierw robi pewne zniszczenia, a następnie radzi sobie podobnie jak w konstruktorze kopiującym.Unikaj powtarzania tego samego kodu w konstruktorze kopiującym i operatorze =

Moje pytanie brzmi: jak napisać to najlepiej, bez powtarzania tych samych linii kodu i bez potrzeby wykonywania niepotrzebnej pracy procesora (jak niepotrzebne kopiowanie).

Zazwyczaj otrzymuję dwie metody pomocy. Jeden do budowy i jeden do zniszczenia. Pierwszy jest wywoływany zarówno z konstruktora kopiowania, jak i operatora =. Drugi jest używany przez destruktor i operator =.

Oto przykładowy kod:

template <class T> 
    class MyClass 
    { 
     private: 
     // Data members 
     int count; 
     T* data; // Some of them are dynamicly allocated 
     void construct(const MyClass& myClass) 
     { 
      // Code which does deep copy 
      this->count = myClass.count; 
      data = new T[count]; 
      try 
      { 
       for (int i = 0; i < count; i++) 
        data[i] = myClass.data[i]; 
      } 
      catch (...) 
      { 
       delete[] data; 
       throw; 
      } 
     } 
     void destruct() 
     { 
      // Dealocate all dynamicly allocated data members 
      delete[] data; 
     } 
     public: MyClass(int count) : count(count) 
     { 
      data = new T[count]; 
     } 
     MyClass(const MyClass& myClass) 
     { 
      construct(myClass); 
     } 
     MyClass& operator = (const MyClass& myClass) 
     { 
      if (this != &myClass) 
      { 
       destruct(); 
       construct(myClass); 
      } 
      return *this; 
     } 
     ~MyClass() 
     { 
      destruct(); 
     } 
    }; 

Czy to jeszcze jest prawidłowy? A czy dobrze jest podzielić kod w ten sposób?

+0

+1, ponieważ pytanie pomogło zwiększyć moją świadomość. Wygląda to na coś, co napisałbym przed przeczytaniem odpowiedzi. – ToolmakerSteve

+0

Hmm, rzadko mam zduplikowany kod w obu, ponieważ oba robią zupełnie różne rzeczy: jeden z nich inicjuje, jeden przypisuje ... – PlasmaHH

+0

jego "głęboką kopię" charakteru jego projektu klasowego, który prowadzi do powielania. – ToolmakerSteve

Odpowiedz

0

Wykonaj zadanie, najpierw kopiując prawą stronę, a następnie zamieniając ją. W ten sposób uzyskasz także wyjątkowe bezpieczeństwo, którego powyższy kod nie zapewnia. Mogłoby się skończyć z uszkodzonym kontenerem, gdy construct() kończy się niepowodzeniem po destruct() powiodło się inaczej, ponieważ wskaźnik elementu odwołuje się do niektórych zwalnianych danych, a także do destrukcji, która zostanie ponownie przydzielona, ​​powodując niezdefiniowane zachowanie.

foo& 
foo::operator=(foo const& rhs) 
{ 
    using std::swap; 
    foo tmp(rhs); 
    swap(*this, tmp); 
    return *this; 
} 
+0

Jeśli konstrukcja się nie powiedzie, dlaczego lepiej jest skończyć ze starym (i nie jest już przeznaczone) treści zamiast pustego kontenera? IMHO byłoby czystsze, gdyby wynik nieudanej kopii był pusty. W szczególności pusty kontener można łatwo wykryć w późniejszym kodzie; Kontener z zawartością, KTÓRA NIE MOŻE BYĆ DŁUŻSZY, jest trudniejszy do wykrycia później. – ToolmakerSteve

+0

Zakładając, że chcesz zakończyć program w przypadku takiej awarii, w każdym przypadku działa dobrze, ale nadal musisz ustawić 'data = nullptr' (którego oryginalny kod nie robi). – riv

+0

@ToolmakerSteve To zależy od tego, co chcesz. Idiom wymiany zapewnia pełną integralność transakcyjną; albo ci się uda, albo nic się nie zmieni. W większości przypadków pełna integralność transakcyjna nie jest potrzebna w przypadku pojedynczych obiektów; wszystko, co musisz zagwarantować, to to, że przedmiot może zostać zniszczony w przypadku awarii. (Próba zagwarantowania jakiegokolwiek stanu rzeczywistego innego niż niezmieniony prawdopodobnie nie jest zbyt przydatna.) –

0

nie widzę żadnego problemu z naturalną, że tak długo, jak upewnić się, że nie zadeklarować konstruktu lub zniszczeniu wirtualny.

Być może zainteresuje Cię rozdział 2 w Effective C++ (Scott Meyers), który jest całkowicie poświęcony konstruktorom, operatorom kopii i destruktorom.

Co do wyjątków, których kod nie obsługuje tak, jak powinien, należy rozważyć elementy 10 & 11 w bardziej efektywnym C++ (Scott Meyers).

+1

Poza tym, że nie jest bezpieczny w wyjątkach. Jeśli 'nowy' w' konstrukcji' rzuca (lub rzutki kopiowania), to obiekt pozostaje w stanie niespójnym, w którym jego destrukcja spowoduje niezdefiniowane zachowanie. –

+0

@JamesKanze Oczywiście masz rację, ale pytanie dotyczy techniki unikania duplikacji kodu, a ta technika nie ma problemów nieodłącznych, jak sądzę. –

+0

Nie ma nieodłącznych problemów, z wyjątkiem tego, że nie działa. Bezpieczeństwo wyjątków nie jest funkcją opcjonalną; jest konieczne, aby program był poprawny. –

5

Jeden Początkowy Komentarz: operator= robi nie początek przez niszczącej, lecz poprzez budowę. W przeciwnym razie obiekt pozostanie w niepoprawnym stanie, jeśli konstrukcja zostanie zakończona z powodu wyjątku od . Twój kod jest nieprawidłowy z tego powodu. (Zauważ, że konieczność przetestowania dla przypisania siebie jest zwykle oznaką, że operator przypisania jest nie prawidłowe.)

Klasycznym rozwiązaniem do obsługi to jest idiom zamiany: ty dodać swap funkcji użytkownika:

void MyClass:swap(MyClass& other) 
{ 
    std::swap(count, other.count); 
    std::swap(data, other.data); 
} 

który jest gwarantowany, aby nie rzucać. (Tutaj, po prostu zamienia int i wskaźnik, którego nie można wyrzucić). Następnie zaimplementować operator przypisania jako:

MyClass& MyClass<T>::operator=(MyClass const& other) 
{ 
    MyClass tmp(other); 
    swap(tmp); 
    return *this; 
} 

To jest proste i prosto do przodu, ale każde rozwiązanie, w którym wszystko operacje, które mogą się nie powieść, są kończone przed rozpoczęciem zmiana danych jest akceptowalna.Dla prostego przypadku podobnego kodu , na przykład:

MyClass& MyClass<T>::operator=(MyClass const& other) 
{ 
    T* newData = cloneData(other.data, other.count); 
    delete data; 
    count = other.count; 
    data = newData; 
    return *this; 
} 

(gdzie cloneData jest funkcją członka, który ma większość co swoją construct ma, ale zwraca wskaźnik, a nie zmodyfikować coś w this).

EDIT:

nie związane bezpośrednio z początkowego pytania, ale ogólnie, w takich przypadkach zrobić nie chcą zrobić new T[count] w cloneData (lub construct, lub cokolwiek). To konstruuje wszystkie z T z domyślnym konstruktorem, a następnie przypisuje je. idiomatyczne sposobem osiągnięcia tego celu jest coś takiego jak:

T* 
MyClass<T>::cloneData(T const* other, int count) 
{ 
    // ATTENTION! the type is a lie, at least for the moment! 
    T* results = static_cast<T*>(operator new(count * sizeof(T))); 
    int i = 0; 
    try { 
     while (i != count) { 
      new (results + i) T(other[i]); 
      ++ i; 
     } 
    } catch (...) { 
     while (i != 0) { 
      -- i; 
      results[i].~T(); 
     } 
     throw; 
    } 
    return results; 
} 

Najczęściej będzie to zrobić za pomocą oddzielnego (prywatny) Kierownik Klasa:

// Inside MyClass, private: 
struct Data 
{ 
    T* data; 
    int count; 
    Data(int count) 
     : data(static_cast<T*>(operator new(count * sizeof(T))) 
     , count(0) 
    { 
    } 
    ~Data() 
    { 
     while (count != 0) { 
      -- count; 
      (data + count)->~T(); 
     } 
    } 
    void swap(Data& other) 
    { 
     std::swap(data, other.data); 
     std::swap(count, other.count); 
    } 
}; 
Data data; 

// Copy constructor 
MyClass(MyClass const& other) 
    : data(other.data.count) 
{ 
    while (data.count != other.data.count) { 
     new (data.data + data.count) T(other.date[data.count]); 
     ++ data.count; 
    } 
} 

(i oczywiście, idiom wymiany do zadania). To pozwala na wiele par liczenie/danych bez ryzyka utraty wyjątku bezpieczeństwa.

+0

+1, dzięki @James, lepiej rozumiem ten temat teraz. – ToolmakerSteve

+0

To jest coś rewolucyjnego wartego więcej niż jeden + - "Zauważ, że konieczność testowania w celu samodzielnego przypisania jest zwykle oznaką, że operator przypisania nie jest prawidłowy." – SChepurin

+0

@James Kanze: Jeden przypadek (do którego wpadł współpracownik) ma miejsce, gdy operator przydziału musi wykonać memcpy na jednym z jego zasobów. W takim przypadku samo-przynależność staje się koniecznością, nie? – ForeverLearning

Powiązane problemy