C++assingment operator using destructor and copying constructor

Question

I was working on a class with multiple dynamic fields and I was looking for quick way of coding assignment operator.

So let's say I have some basic class Cla, which stores dynamic array of integers (arr) and the size of said array (n).

I've coded this:

Cla::Cla(int* arr, int n) : n(n)
{
    this->arr = new int[n];

    //allocation error handling

    while (--n >= 0)
        this->arr[n] = arr[n];
}


Cla::~Cla()
{
    delete[] arr;
}

Cla::Cla(const Cla& copy) : Cla(copy.arr, copy.n){}

Cla& Cla::operator= (const Cla& asg)
{
    this->~Cla();
    *this = asg;
    return *this;
}

All of it works properly, except for operator=. The idea was that I'll just destroy my object and then create it again with copying constructor (for the sake of simplicity of the example I don't consider the situation where both objects have the same size and there is no need for deallocation and new allocation). It compiles, but it gives me some nasty errors when executed.

Can you give me some advice on how to correct this code? Is this even possible for it to work this way? (I know how to write an assingment operator, I'm just asking whether it is possible to do it using destructor and copying constructor. I couldn't find anything like that on the internet.)

Does this answer your question? [What is the copy-and-swap idiom?](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) — rsjaffe, Mar 31 '20 at 16:24
Once you've done `this->~Cla();`, the object is destructed. Continuing to use the recently destructed object, even within its own method, is undefined behavior. — Eljay, Mar 31 '20 at 16:38
What do you think this line does? `*this = asg;` in your assignment operator — balki, Mar 31 '20 at 16:41
In general, this is a bad approach. You can't derive from a class that does this. — Pete Becker, Mar 31 '20 at 17:16
@balki well I thought that since the old object was destroyed, it will create a new one using copying constructor. I also tried to do `Cla newCla = asg; *this = newCla` but it didn't work either. — MartinYakuza, Mar 31 '20 at 17:17

Remy Lebeau · Accepted Answer · 2020-03-31T17:48:26.990

Your operator= has undefined behavior. First, you cannot manually call a destructor on an object that was not allocated with placement-new. Second, once an object is destroyed, it cannot be used anymore, which means *this = asg is accessing invalid memory once this->~Cla() has been called, as this is no longer pointing at a valid object. Third, your operator= is running an endless recursion loop, calling itself over and over until the call stack blows up (if you are lucky).

Since you want to use your copy constructor, your operator= would be better served by using the copy-swap idiom instead. Construct a local object to make use of your copy constructor, and then swap the contents of that object with this so that this takes ownership of the copied data and the local object frees the old data when it goes out of scope, eg:

Cla& Cla::operator= (const Cla& asg)
{
    if (&asg != this)
    {
        Cla temp(asg);
        std::swap(arr, temp.arr);
        std::swap(n, temp.n);
    }
    return *this;
}

Alternatively:

void Cla::swap(Cla &other)
{
    std::swap(arr, other.arr);
    std::swap(n, other.n);
}

Cla& Cla::operator= (const Cla& asg)
{
    if (&asg != this) {
        Cla(asg).swap(*this);
    }
    return *this;
}

That being said, the fact that your copy constructor is delegating to your converting constructor means that you are using C++11 or later, in which case you should also implement move semantics into your class, not just copy semantics, eg:

Cla::Cla() : arr(nullptr), n(0)
{
}

Cla::Cla(int* arr, int n) : arr(new int[n]), n(n)
{
    while (--n >= 0)
        this->arr[n] = arr[n];
}

Cla::Cla(Cla &&c) : arr(nullptr), n(0)
{
    c.swap(*this);
}

Cla::Cla(const Cla& c) : Cla(c.arr, c.n)
{
}

Cla::~Cla()
{
    delete[] arr;
}

void Cla::swap(Cla &other)
{
    std::swap(arr, other.arr);
    std::swap(n, other.n);
}

Cla& Cla::operator= (Cla asg)
{
    asg.swap(*this);
    return *this;
}

By passing the asg parameter by value, operator= can decide whether to use copy semantics or move semantics at the call site based on whether an lvalue or rvalue is being passed to it. The compiler will pick the appropriate constructor to construct the asg parameter with, and then this can take ownership of the resulting data.

Thank you! The first paragraph of your answer already solves my question, as I said I know how to write this operator, I was just asking whether I can write it in this specific way. Anyways, thank you. — MartinYakuza, Mar 31 '20 at 22:04

C++assingment operator using destructor and copying constructor

1 Answers1