square1001
square1001

Reputation: 1464

How to deal with double (or multiple) copy constructors for class with pointers

There is a class called "X" like this:

class X {
private:
    int *ptr;
public:
    X() {
        ptr = new int[2];
        ptr[0] = 0;
        ptr[1] = 0;
    }
    X(int a, int b) {
        ptr = new int[2];
        ptr[0] = a;
        ptr[1] = b;
    }
    X(const X &val) {
        delete[] ptr;
        ptr = new int[2];
        ptr[0] = val.ptr[0];
        ptr[1] = val.ptr[1];
    }
    X get() {
        X ret(ptr[0], ptr[1]);
        return ret;
    }
};

Suppose that there are variable X that is defined with X v(2, 3).

If we call v.get(), it's okay.

But, if we call v.get().get(), it's not okay. It produces runtime error in delete[] ptr section in copy constructor, which the content is that I am trying to delete the undefined (0xcccccccc) pointer.

One possible option for dealing with this is, that using C++-STL like <array> or <vector>. But I want to implement with pointer.

How to deal with this runtime error, with pointer-based implementation?

Upvotes: 0

Views: 80

Answers (1)

Caleth
Caleth

Reputation: 62704

The simple answer is that you shouldn't delete[] ptr in the copy constructor, because it is uninitialised. You don't need to delete[] ptr in assignment, either. The only place delete or delete[] should occur is in a destructor.

Tidying up your class, without changing from owning raw pointers

class X {
private:
    int *ptr;
public:
    X() : X(0, 0) { }
    X(int a, int b) : ptr(new int[2]) {
        ptr[0] = a;
        ptr[1] = b;
    }
    X(const X &val) : X(val.ptr[0], val.ptr[1]) { }
    X& operator=(const X &val)
    {
        ptr[0] = val.ptr[0];
        ptr[1] = val.ptr[1];
        return *this;
    }
    X(X&& val) : ptr(val.ptr) {
        val.ptr = nullptr;
    }
    X& operator=(X&& val) {
        std::swap(ptr, val.ptr);
        return *this;
    }        
    ~X() { delete[] ptr; }

    X get() {
        return *this;
    }
};

As a point of nomenclature, a constructor is only a "copy constructor" if it takes an instance of the class as it's single (non-default) parameter. X::X(int, int) is just a constructor. That means there are at most 4 copy constructors (otherwise they would be ambiguous under overload resolution)

X::X(X &)
X::X(const X &)
X::X(volatile X &)
X::X(const volatile X &)

However defining more than one is a terrible idea, unless you in an obfuscation contest

Upvotes: 1

Related Questions