Должен ли я удалить указатель начала в функции назначения оператора?

Question

Я написал следующий демо-код, чтобы узнать, как работать с конструктором и оператором присваивания. Однако есть немного путаницы. Мне сказали удалить указатели в операторе присваивания и выделить новый адрес data. Однако я могу только заставить мой код работать, удаляя эту строку. Я взял страницу this как ссылку, но она показывает только пример int, а не int*. Как я могу решить эту проблему?

#include <iostream> 
#include <string> 
#include <vector> 
#include <random> 
#include <boost/smart_ptr.hpp> 
#include <boost/make_shared.hpp> 

using namespace boost; 

class ClassOne 
{ 
public: 
    ClassOne():data(NULL) {} 
    ClassOne(int data_param):data(NULL) 
    { 
     init(data_param); 
     std::cout << "construct" << std::endl; 
    } 

    virtual ~ClassOne() 
    { 
     if (data) { 
      delete data; 
     } 
     data = NULL; 
    } 

    ClassOne(const ClassOne& rhs){ 
     std::cout<< "copy " <<std::endl; 
     data = NULL; 
     init(*rhs.data); 
    } 

    ClassOne& operator = (const ClassOne& rhs){ 
     std::cout<< "assign " <<std::endl; 
     int* p_old = rhs.data; 
     data = new int(*p_old); 
     //delete p_old;   // I have to delete this line to make my code work 
     return *this; 
    } 

    void init(int data_param) 
    { 
     if (data) { 
      delete data; 
     } 
     data = new int(data_param); 
    } 

private: 
    int* data; 
}; 

int main(int argc, const char * argv[]) { 
    ClassOne c1(10); 
    ClassOne c2(c1);  // call copy constructor 
    ClassOne c3; 
    c3 = c1;    // call assignment function 
    return 0; 
} 

Answer 1

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

ClassOne& operator = (const ClassOne& rhs){ 
    std::cout<< "assign " <<std::endl; 
    delete data;    // delete your own old data 
    data = new int(*rhs.data); // clone the rhs's data 
    return *this; 
} 

Answer 2

Ваш код не потому, что вы что-то двойное УДАЛЕНИЕ: вы удаляете указатель в назначении копирования и деструктор. Удаление указателя не делает его нулевым, поэтому он передает значение if в вашем деструкторе. (Кстати, вам не нужно проверить указатель утратившими перед удалением, delete делает чек в любом случае)

Я рекомендовал бы назначение копирования, чтобы не изменить переменную rhs, поскольку удаление data указателя может легко приводят к доступу к памяти. Я предпочел бы реализовать конструктор перемещения и назначение, чтобы сделать это поведение явным. Я хотел бы удалить конструктор копирования и назначение и добавить эти функции:

ClassOne(const ClassOne&) = delete; 
ClassOne& operator=(const ClassOne&) = delete; 

ClassOne(ClassOne&& rhs) { 
    std::swap(data, rhs.data); 
} 

ClassOne& operator=(ClassOne&& rhs) { 
    std::swap(data, rhs.data); 
} 

Это потребует std::move быть звоните по заданию.

В качестве альтернативы вы можете реализовать конструктор без кражи. Это потребовало бы, чтобы youto глубокой копии указателя данных (копирование содержимого вместо указателя)

ClassOne(const ClassOne& rhs) { 
    if (!data) { 
     data = new int; 
    } 
    *data = *(rhs.data); 
} 

ClassOne& operator=(const ClassOne& rhs) { 
    if (!data) { 
     data = new int; 
    } 
    *data = *(rhs.data); 
}