Destructor issue

Question

Which is the difference between these two codes? Is there a memory leak in the first case?

no destructor defined

 class Library
{
private:
    Book books[50];
    int index;
public:
    Library()
    {

        index=0;
    }
};

or with destructor defined

class Library
{
private:
    Book *books;
    int index;
public:
    Library()
    {
        books=new Book[50];
        index=0;
    }
    ~Library()
    {
        delete books;
    }
};

The second has UB. The first has no memory leak. The second also brings in the rule of three/five. — chris, Nov 07 '12 at 20:50
i am a beginner with oop and i don't know what UB means or which is the rule three/five — laura, Nov 07 '12 at 20:54
"Rule of three C++" can be easily googled -- and UB means "Undefined behavior", ie your compiler could choose to blow up your house in response to what you are doing, and it would be standards-compliant. — Yakk - Adam Nevraumont, Nov 07 '12 at 21:00
[Rule of Three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three), [Rule of Five](http://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11), [Rule of Zero](http://rmartinho.github.com/2012/08/15/rule-of-zero.html), [Different Kinds of Behaviour](http://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior). — chris, Nov 07 '12 at 21:01
Also, prefer using member initializers in constructors instead of assignment. E.g., `Library() : index(0) {}` rather than `Library() { index = 0; }`. — Fred Larson, Nov 07 '12 at 21:08
Looks like you haven't read your C++ book all the way through yet. — Lightness Races in Orbit, Nov 07 '12 at 21:12

score 3 · Accepted Answer · answered Nov 07 '12 at 20:53

delete books;

should be

delete[] books;

In the second one: books is allocated as an array, so must be deleted as an array. The first version does not have a memory leak.

The difference is that in the first case the memory for books is is contained within the allocation for each instance of Library, wheras in the second case it is allocated separately, using the heap.

score 1 · Answer 2 · edited Nov 07 '12 at 21:03

First off, the second code should be

~Library() {
  delete[] books;
}

new must be matched with delete, new[] must be matched with delete[].

In the first code, every instance of Library contains 50 instances of Book. Copying a Library will copy those 50 instances. There is no memory leak.

In the second code, an instance of Library contains just a pointer to the books. Copying the instance (without the default copy constructor) copies just the pointer - the original and copy will share the 50 books, and whoever is destroyed second will issue delete on already deleted memory, which is an error.

score 1 · Answer 3 · answered Nov 07 '12 at 21:01

~Library()
{
    delete[] books;
}

is a proper solution. There are free possible problems in your code, which you can consider:

1) use initialization list

in second case:

2) Do you really need to do pointer? isnt a local object enough?( i guess your books doesnt exceed stack size). Look at your code you are creating it at the beginning and delete at the end. Remember stack is faster and more safety.

Also be familiar with valgrind, its a really great tool. Check if cppcheck will help you. "sudo apt-get install cppcheck" in ubuntu.

Destructor issue

3 Answers3