CODEWITHSUNDEEP
c++destructor
mja
mja

Reputation: 1365

C++: Do I need "if" statement for destructor?

I think my destructor of Vector class below should have if statement to cancel the memory that it uses. If arr has one member, it will have delete arr. And if arr has many members, I must use delete[] arr.

Could you tell me that is necessary?

My code:

class Vector {
    double * arr;
    short dim;

public:
    Vector(short d = 0): dim(d) {
        arr = NULL;
        if (dim < 0) {
            dim = 0;
        } else {
            arr = new double[dim];
        }
    }

    ~Vector() {
        if (arr != NULL) {
            if (dim == 1) {
                delete arr;
            } else {
                delete[] arr;
            }

            arr = NULL;
            dim = 0;
        }
    }
};

Upvotes: 2

Views: 904

Answers (4)

Dota
Dota

Reputation: 53

No this is not necessary. Make sure when you create allocated memory using new keyword to delete the memory without square brackets avoid mixing them.

Upvotes: 2

NathanOliver
NathanOliver

Reputation: 180500

Not only is this not necessary but this is illegal since you only allocate memory with new[]. If you call new you need a delete, if you call new[] you need a call to delete[]. Mixing them is undefined behavior. Your vector it should look something like:

class Vector {
    double * arr;
    short dim;

public:
    Vector(short d = 0): dim(d) {
        if (dim > 0)
            arr = new double[dim]
        else
            arr = nullptr;
    }

    Vector(const Vector& copy) : dim(copy.dim) {
        if (dim > 0) {
            arr = new double[dim]
            // copy data here
        }
        else
            arr = nullptr;
    }

    ~Vector() {
        delete [] arr;
    }

    Vector & operator=(Vector rhs) {
        // swap the contents of the copy.  you can make a swap function to do this
        double * temp = arr;
        arr = rhs.arr;
        rhs.arr = temp;
        dim = rhs.dim;
    }

};

Now we have correct copies and delete will either be a non-op on a nullptr or correctly deallocate the memory allocated from the constructor.

Upvotes: 6

πάντα ῥεῖ
πάντα ῥεῖ

Reputation: 1

You never call new double; anywhere, so it's not needed. Always call delete [] arr;. Check for NULL isn't needed also.

Upvotes: 4

HolyBlackCat
HolyBlackCat

Reputation: 96126

Not only it's not necessary, it is just incorrect. Anything that was created with new [] can only be deleted with delete []. Otherwise it creates an undefined behavior.

Upvotes: 10

Related Questions