Search code examples
c++copy-constructorassignment-operator

Should I delete origin pointer in operator assignment function?


I have write following demo code to learn copy constructor and assignment operator. However there is a little confuse. I was told to delete pointers in assignment operator and allocate new address to the data. However I can only make my code work delete that line. I have taken this page as a reference, but it only shows the example of int while not the int*. How can I solve this problem?

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

Solution

  • You are trying to delete the other object's data member, when you are meant to delete your own (this) object's current data member instead. What you probably want is 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;
    }