5

В C++, когда классы содержат динамически распределенные данные, обычно разумно явно определять конструктор копирования, operator = и destructor. Но деятельность этих специальных методов перекрывается. Более конкретно оператор = обычно сначала выполняет некоторую разрушение, а затем справляется с аналогичной задачей в конструкторе копирования.Избегайте повторения одного и того же кода в конструкторе копирования и операторе =

Мой вопрос заключается в том, как написать это наилучшим образом, не повторяя одни и те же строки кода и не требуя, чтобы процессор выполнял ненужную работу (например, ненужное копирование).

Обычно я получаю два метода помощи. Один для строительства и один для уничтожения. Первый вызван как из конструктора копирования, так и для оператора =. Второй используется деструктором и оператором =.

Вот пример кода:

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

Является ли это даже правильно? И это хорошая привычка разбивать код таким образом?

+0

+1 потому что вопрос помог поднять мое осознание. Это похоже на то, что я написал бы, прежде чем читать ответы. – ToolmakerSteve

+0

Хм, у меня редко есть дублированный код в обоих случаях, так как они оба делают совершенно разные вещи: один intializes, один присваивает .... – PlasmaHH

+0

его «глубокая копия» характера его класса, который приводит к дублированию. – ToolmakerSteve

ответ

0

Внесите присвоение, сначала скопировав правую часть, а затем заменив ее. Таким образом, вы также получаете исключительную безопасность, которую ваш код выше не предоставляет. Вы можете завершить разбитый контейнер, когда construct() завершится с ошибкой после того, как destruct() будет выполнен в противном случае, потому что указатель-член ссылается на некоторые освобожденные данные и на уничтожение, которое будет освобождено снова, что приведет к неопределенному поведению.

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

Если конструктор выходит из строя, почему лучше в конечном итоге со старым (и более не предназначенным), а не с пустым контейнером? ИМХО было бы чище иметь неудачный результат копирования в пустом контейнере. В частности, пустой контейнер легко обнаружить в более позднем коде; контейнер, у которого есть содержимое, НЕ ДОЛЖНО НИКОГДА НЕ ДОЛЖНО обнаруживать позже. – ToolmakerSteve

+0

Предполагая, что вы хотите завершить программу в случае такого сбоя, любой способ работает нормально, но вам все равно нужно установить 'data = nullptr' (который не удается выполнить из исходного кода). – riv

+0

@ToolmakerSteve Это зависит от того, что вы хотите. Идиома свопа обеспечивает полную целостность транзакций; вы либо преуспеваете, либо ничего не меняется. В большинстве случаев полная целостность транзакций не требуется для отдельных объектов; все, что вы должны гарантировать, заключается в том, что объект может быть разрушен в случае сбоя. (Попытка гарантировать какое-либо фактическое состояние, отличное от неизмененного, вероятно, не очень полезно.) –

0

Я не вижу в себе неотъемлемой проблемы, если вы не должны объявлять конструкцию или разрушать виртуальные.

Возможно, вас заинтересует глава 2 в Effective C++ (Scott Meyers), которая полностью посвящена конструкторам, операторам копирования и деструкторам.

Что касается исключений, которые ваш код не обрабатывает, как следует, рассмотрите пункты 10 & 11 в более эффективном C++ (Scott Meyers).

+1

За исключением того, что это не исключение. Если 'new' в' construct' бросает (или копирует броски), то объект остается в некогерентном состоянии, в котором разрушение приведет к неопределенному поведению. –

+0

@JamesKanze Конечно, вы правы, но вопрос в том, как избежать дублирования кода, и эта техника не имеет неотъемлемых проблем, я думаю. –

+0

У него нет неотъемлемых проблем, кроме того, что он не работает. Безопасность исключений не является необязательной функцией; важно, чтобы программа была правильной. –

5

Одним из первых шагов Комментарий: operator= делает не старт на разрушающих, но строительство. В противном случае он оставит объект в недопустимом состоянии, если строительство завершается с помощью исключения . Из-за этого ваш код неверен. (Обратите внимание, что необходимость проверки для самостоятельного присвоения, как правило, признак того, что оператор присваивания не правильно.)

Классическое решение для обработки это своп идиома: вы добавить функцию-член свопа:

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

который гарантированно не выбрасывает. (Здесь, он просто меняет местами в Int и указатель, ни один из которых может бросить.) Тогда вы реализовать оператор присваивания, как:

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

Это очень просто и прямо вперед, но любое решение, в котором все операции, которые могут завершиться неудачно, завершены до того, как вы начнете изменение данных допустимо.Для простого случая, как ваш кода, например:

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

(где cloneData функция элемента, который делает большую часть того, что вашего construct делает, но возвращает указатель, и ничего не изменения в this).

EDIT:

не связаны с вашим первоначальным вопросом, но, как правило, в таких случаях , вы не хотите сделать new T[count] в cloneData (или construct, или любой другой). Это создает все из T со стандартным конструктором и затем назначает их. идиоматических способ сделать это что-то вроде:

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

Чаще всего, это будет сделано с помощью отдельного (частного) менеджеру класс:

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

(и, конечно, своп идиома для назначения). Это позволяет использовать несколько пар count/data без риска потери исключения. безопасность.

+0

+1, спасибо @James, я лучше понимаю эту тему сейчас. – ToolmakerSteve

+0

Это что-то революционное стоит больше одного + - «Обратите внимание, что необходимость проверки самоопределения обычно является признаком того, что оператор присваивания неверен». – SChepurin

+0

@James Kanze: Один случай (с которым столкнулся коллега) - это когда ваш оператор назначения должен сделать memcpy на одном из своих ресурсов. В этом случае самоназвание становится необходимостью, нет? – ForeverLearning