Writing a destructor for a class containing a dynamically allocated array member c++

Question

(C++ code) Suppose I have a class like:

class A {
public: 
   int * ptr;
   int size;
    A();
    ~A();
}

A::A()
{
    ptr = new int[10];  // memory allocation is dependent upon user input into size variable 
}
A::~A()
{ 
    delete [] ptr; 
}

According to what I could find (sample link: How delete a pointer of classes which has pointer members?) this seems to be the correct way of defining the destructor here. But I tried this and it runs into run time exception. Even tried, delete ptr; but no luck

Is this the correct way of doing things?

The array based implementation of queue :

#include<iostream>
#include<string>
using namespace std;

class ArrayQueue
{
public:
    int front, size, rear, curr;
    int * elem;
    ArrayQueue():size(10),rear(-1),curr(0),front(-1){}
    ArrayQueue(int n);
    void enqueue(int& e);
    void dequeue();
    bool empty()const;
    int& getFront() const;
    int& getRear() const;
    int& getSize();
    ~ArrayQueue();

};

ArrayQueue::ArrayQueue(int n)
{
    size = n;
    front = -1;
    rear=-1;
    curr=0;
    elem = new int[size];
}
ArrayQueue::~ArrayQueue()
{  cout<<"running destructor";
    delete[] elem;
}

bool ArrayQueue::empty()const
{
    return  curr==0;
}
int& ArrayQueue::getSize()
{
    return curr;
}
int& ArrayQueue::getFront()const
{
    return elem[front];
}
int& ArrayQueue::getRear()const
{
    return elem[rear];
}
void ArrayQueue::enqueue(int& e)
{
    if(getSize()==size){cout<<"Queue full"<<'\n'; return;}
    rear = ((rear+1)%size);
    elem[rear]=e;
    ++curr;
}
void ArrayQueue::dequeue()
{
    if(empty()){cout<<"Queue empty"<<'\n';return;}
    front = ((front+1)%size);
    --curr;
}


int main()
{
    ArrayQueue object;
    cout<<"Size of the queue"<<object.size<<'\n';
    cout<<"Current size:"<<object.getSize()<<'\n';
    object.dequeue();
    for(int i =0 ; i<15; i++)
    {
        object.enqueue(i);
    }
    cout<<"front element : "<<object.front<<'\n';
    cout<<"rear element : "<<object.rear<<'\n';
    cout<<"Current size:"<<object.getSize()<<'\n';
    object.dequeue();
    object.dequeue();
    object.dequeue();
    cout<<"front element : "<<object.front<<'\n';
    cout<<"rear element : "<<object.rear<<'\n';
    return 0;
}

Search for "Rule of 3" (Maybe "Rule of 5") and "Rule of 0". You have either too few special member-functions overridden, or too many (in the latter case, you should use a smart-pointer like `std::unique_ptr` though, no overhead, what a deal). On second thought, always use smart-pointers, whatever else you change. — Deduplicator, Nov 12 '14 at 20:39
The code you've presented here does not contain any errors as such. But if you ever copy objects of `A` around, you'll run into trouble (since you didn't overload the copy constructor, the default one is used and then two objects have the same pointer and it gets double-freed (and freed before it's done being used)). I suspect that's what's happening in your real code. — Cameron, Nov 12 '14 at 20:47
@Cameron In my actual code I only define a single object for the class, so there is no copying between objects. The code is for a simple array based implementation of queue. — tinus91, Nov 12 '14 at 20:51
_when_ is there a runtime exception? Are you sure it's on destruction of `ArrayQueue`? — David, Nov 12 '14 at 21:00

davidhigh · Accepted Answer · 2014-11-12T23:38:22.183

Is this the correct way of doing things?

No, not in modern C++. For example, here is one problem: if you copy the class A as it is, you make a so-called shallow copy and copy only the pointer. If now one of these objects goes out of scope, it deletes the array ptr, with the effect that the copied class can not access it anymore.

This is why there is something like the "rule of three". It states that if you need a destructor, it is likely that you handle the memory layout by yourself and therefore also require (at least) a copy constructor and an assignment operator. In your case, such a copy constructor would make a "deep copy" and set up a new array in the copied class, which contains the same elements as the array in the original class.

But all this can be avoided in modern C++.

The way which is considered correct there is namely to use members which manage their memory allocation itself. The most native way of doing what you intended is to use a std::array or a std::vector instead of the C-style array.

struct A
{
   //everything is handled correctly here without destructor, copy constructor, etc.
   //this is an example of RAII or the rule of zero 
   std::vector<int> v;
}

Copies, destruction and assignment is now performed by calling the corresponding copy constructor, destructor and assignment operator of std::vector, which you can be sure of it is implemented correctly. So, if you make a copy of your class, you create two different objects of std::vector.

Another option is to use a std::shared_ptr.

struct A
{
   std::shared_ptr<std::vector<int> > v;   //you could also use an int* here,
                                           //but always prefer std::vector 
}

Again, you don't need to care for the implementation of the copy, destructor or assignment, as you can rely on the default implementations for them.

If you make a copy of your class A, it is again a shallow copy, i.e. you don't copy the object pointed to by the shared_ptr, but only the pointer. However, now the shared_ptr counts the number of instances pointing to the managed std::vector and prevents the copied class from deleting the managed object if it goes out of scope.

Which of these alternatives you need depends on your requirements. In each case, you should make yourself familiar with these concepts, as in my opinion they are very important in C++.

score 1 · Answer 2 · answered Nov 12 '14 at 21:03

1

In your code you don't call ArrayQueue::ArrayQueue(int n), instead you call the default constructor ArrayQueue::ArrayQueue() in which you don't allocate memory for elem.

answered Nov 12 '14 at 21:03

Anton Savin

40,838
8
54
90

Writing a destructor for a class containing a dynamically allocated array member c++

2 Answers2