delete vs delete[] in destructor

Question

I study for an exam in c++ and I was asked if in this code the d'tor of the class should use delete[] instead delete:

    template <class T>
class ClonePtr
{
private:
    T* ptr;
public:
    explicit ClonePtr(T* p = nullptr) : ptr(p) {}
    ~ClonePtr() { if(ptr!=nullptr) delete []ptr; }
    ClonePtr(const ClonePtr& other) : ptr(nullptr)
    {
        *this = other;
    }
    ClonePtr(ClonePtr&& other) : ptr(other.ptr)
    {
        other.ptr = nullptr;
    }
    ClonePtr& operator=(const ClonePtr& other)
    {
        if (this != &other)
        {
            delete ptr;
            if (other.ptr != nullptr)
                ptr = other.ptr->clone();
        }
        return *this;
    }
    T& operator*() const { return *ptr; }
};

The right answer to the question is yes, but why is that?

+I have two more little question regarding this code:

1. It says in the exam that type T must be a class and cannot be a primitve type. Why is that?
2. On the other hand, I tried writing this code:

class A
{
private:
    int x;
public:
    A(int x = 8) : x(x) { };
};

int main()
{
    ClonePtr<int> h(new A(7));
}

and got a compiler error: C2664 "cannot convert argument 1 from A* to T*"

I will very much appreciate your help.

thanks :)

Answer 1

There is no way for us to answer this definitely one way or the other, because we can't see what kind of pointer is being passed to ClonePtr(T*), or what kind of pointer is being returned by clone(). This class is not doing any memory allocations of its own.

IF both pointers are being allocated with new and not by new[], then delete would be the correct answer, not delete[].

IF both pointers are being allocated with new[] and not by new, then delete[] would be the correct answer, not delete.

For that matter, since this code is taking ownership of memory allocated by something else, and it is clear by this class' use of nullptr and a move constructor (but not a move assignment operator!) that you are using C++11 or later, so you should be utilizing proper ownership semantics via std::unique_ptr or std::shared_ptr, not using a raw pointer at all.

Especially since your copy assignment operator is not setting ptr to nullptr after delete ptr; if other.ptr is nullptr, leaving *this in a bad state. Using proper ownership semantics would have avoided that.

Try this instead:

---

UPDATE: now you have posted additional code (that doesn't compile, since an A* can't be assigned to an int*, and A does not implement clone()) showing ptr being set to an object allocated with new, so the correct answer is:

• delete MUST be used, not delete[].

So the "right answer" to your exam is wrong.

But proper ownership semantics would be even better, eg:

#include <memory>

template <class T>
class ClonePtr
{
private:
    std::unique_ptr<T> ptr;

public:
    ClonePtr(std::unique_ptr<T> p) : ptr(std::move(p)) {}

    ClonePtr& operator=(ClonePtr other)
    {
        ptr = std::move(other.ptr);
        return *this;
    }

    T& operator*() const { return *ptr; }
};

class A
{
private:
    int x;

public:
    A(int x = 8) : x(x) { };

    std::unique_ptr<A> clone() const
    {
        return std::make_unique<A>(x);
        // or: prior to C++14:
        // return std::unique_ptr<A>(new A(x));
    }
};

int main()
{
    ClonePtr<A> h(std::make_unique<A>(7));
    // or, prior to C++14:
    // ClonePtr<A> h(std::unique_ptr<A>(new A(7)));
}