Использование конструктора копирования в операторе присваивания

Question

Это противоречит руководству по стилю использовать конструктор копирования в операторе присваивания? То есть:

const Obj & Obj::operator=(const Obj & source)
{
    if (this == &source)
    {
        return *this;
    }

    // deep copy using copy-constructor
    Obj * copy = new Obj(source);

    // deallocate memory
    this->~Obj();

    // modify object
    *this = *copy;

    return *copy;
}

Предполагая, что конструктор копирования выполняет глубокое копирование объекта.

---

РЕДАКТИРОВАТЬ:

Мой код крайне ошибочен, как отмечают комментаторы.

Что касается общего концептуального вопроса: как предположил WhozCraig, идиома копирования / обмена, похоже, является подходящим способом: что такое идиома копирования и обмена?

Answer 1

Вот вкратце идиома копирования / замены на вашем примере:

#include <algorithm>

class Obj
{
    int *p;
    void swap(Obj& left, Obj& right);

public:
    Obj(int x = 0) : p(new int(x)) {}
    Obj(const Obj& s);
    Obj& operator = (const Obj& s);
    ~Obj() { delete p; }
};

Obj::Obj(const Obj& source) : p(new int(*source.p))
{}

void Obj::swap(Obj& left, Obj& right)
{
    std::swap(left.p, right.p);
}

Obj & Obj::operator=(const Obj & source)
{
    Obj temp(source);
    swap(*this, temp);
    return *this;
}

int main()
{
    Obj o1(5);
    Obj o2(o1);
    Obj o3(10);
    o1 = o3;
}

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

Если вы сосредоточитесь на операторе присваивания, он вызывает Obj Копировать конструктор для создания временного объекта. Тогда Obj-конкретный swap называется, что меняет отдельных членов. Теперь магия в temp объект после swap называется.

Когда деструктор temp называется, будет звонить delete на значение указателя, что this Раньше, но был поменялся с temp указатель. Так когда temp выходит из области видимости, очищается память, выделенная "старым" указателем.

Также обратите внимание, что во время задания, если new выдает исключение во время создания временного объекта, назначение будет генерировать исключение перед любым членом this стать измененным. Это препятствует тому, чтобы у объекта были члены, которые могут быть повреждены из-за их непреднамеренного изменения.

Теперь был дан предыдущий ответ, который использует часто используемый подход "общего кода" для копирования назначения. Вот полный пример этого метода и объяснение причин его возникновения:

class Obj
{
    int *p;
    void CopyMe(const Obj& source);

public:
    Obj(int x = 0) : p(new int(x)) {}
    Obj(const Obj& s);
    Obj& operator = (const Obj& s);
    ~Obj() { delete p; }
};

void Obj::CopyMe(const Obj& source)
{
    delete p;
    p = new int(*source.p);
}

Obj::Obj(const Obj& source) : p(0)
{
   CopyMe(source);
}

Obj & Obj::operator=(const Obj & source)
{
    if ( this != &source )
        CopyMe(source);
    return *this;
} 

Таким образом, вы бы сказали "что не так с этим?" Ну, дело в том, что это неправильно CopyMeпервое, что он делает, это позвонить delete p;, Тогда следующий вопрос, который вы зададите: "Ну и что? Разве это не то, что мы должны делать, удалить старую память?"

Проблема в том, что существует возможность для последующего вызова new терпеть неудачу. Поэтому мы уничтожили наши данные еще до того, как узнали, что новые данные будут доступны. Если new теперь выдает исключение, мы испортили наш объект.

Да, вы можете легко исправить это, создав временный указатель, выделив и в конце назначив временный указатель p, Но во многих случаях об этом можно забыть, и код, подобный приведенному выше, остается в кодовой базе навсегда, даже если в нем есть потенциальная ошибка, связанная с повреждением.

Чтобы проиллюстрировать, вот исправление для CopyMe:

void Obj::CopyMe(const Obj& source)  
{
    int *pTemp = new int(*source.p);
    delete p;
    p = pTemp;
}

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

Answer 2

То, что вы пытаетесь сделать, не работает. Кажется, вы думаете, что объект, возвращаемый оператором присваивания, становится новым "this", но это не так. Вы не модифицировали объект, просто уничтожили его.

Obj a;
Obj b;
a = b; // a has been destroyed
       // and you've leaked a new Obj.
// At the end of the scope, it will try to destroy `a` again, which
// is undefined behavior.

Можно реализовать оператор присваивания с помощью размещения new, но это хрупкое решение, и обычно не рекомендуется:

const Obj & Obj::operator=(const Obj & source)
{
    if (this == &source)
    {
        return *this;
    }

    this->~Obj();

    new (this) Obj(source);

    return *this;
}

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

Answer 3

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

Personnaly, я часто делаю это, умно кодируя свой оператор присваивания и затем вызывая его из конструктора копирования:

Obj::Obj(const Obj & source)
{
    Obj::operator=( source );
}

const Obj& Obj::operator=(const Obj& source)
{
    if (this != &source)
    {
        // copy source attribtes to this.
    }    
    return *this;
}

Это работает, если вы пишете operator= правильно. Как прокомментировано, можно порекомендовать использовать функцию подкачки, используемую как: конструктором копирования, так и перегрузкой оператора = в C++: возможна ли общая функция?

В любом случае, ваша идея обмениваться кодом между обеими функциями была хорошей, но то, как вы ее реализовали, - нет. работать наверняка.. у него много проблем и не делать то, что вы хотите. Он вызывает рекурсивно оператор =. Более того, вы никогда не должны явно вызывать функции деструктора, как вы это делали (this->~Obj();).