Does new and delete have to match?

[qmzh85]

Dear all:

I know as a general rule, the occurrence of new and delete should match. If we new once and delete twice, it will lead to disaster.

But I read some code in "Sams Teach Yourself Data Structure and Algorithms in 24 Hours", Hour 3, where the class is defined as:

class ClassDataArray
{
public:
    ClassDataArray(size_t max);         // constructor
    ~ClassDataArray();                  // destructor
    Person* find(string nameToFind);    // find specified value
    void insert(string last, string first, size_t age);     // insert one person into the array
    bool remove(string nameToRemove);
    void displayArray();
private:
    vector<Person*> m_vec;              // vector of pointers
    size_t m_numOfElements;             // number of data items
};

The definitions of the constructor and destructor are given as below.

// Constructor
ClassDataArray::ClassDataArray(size_t max)
{
    m_vec.resize(max);
    m_numOfElements = 0;
}
// Destructor
ClassDataArray::~ClassDataArray()
{
    for (size_t i = 0; i != m_numOfElements; ++i)
        delete m_vec[i];
}

My concern is: The constructor does not use new but the destructor uses delete. Is this okay or a good practice?

Also need to mention: Method insert() uses new and remove() uses delete, so I suppose that should be a matching pair. But that should not have anything to do with the destructor.

Thanks guys, any explanation is appreciated.

[giblit]

Insert and remove are for one Person. When you insert it increases the m_numOfElements and when you remove it will decrease m_numOfElements When the object is out of scope it will delete/remove all the remaining objects that may have been allocated.


Say for example:

You insert 10 people
You remove the first 5 people

There are 5 people left.

The Object goes out of scope (calls destructor)
Now you must delete/deallocate the remaining people. If you did not there would be a memory leak.

[xismn]

If the insert() method only adds a Person pointer if new didn't throw some kind of problematic exception, the usage of delete in the destructor seems perfectly fine to me. Of course, there's always that chance I may be missing something - because I'm also still learning.
To explain why it seems fine: Each element of the vector whose elements are getting deleted in the destructor would guarantee to point to valid new'd memory, if insert can guarantee that only successfully new'd objects are added (duh).

*EDIT* giblit is right. It would probably be safer to remove the m_numOfElements member, and simply use the m_vec.size() method - there's a chance that you may forget to update the m_numOfElements member.

[qmzh85]

Many thanks to you guys. I think I understand a little bit deeper.

So here is my understanding:
In my simple case, even the constructor does not use new, destructor still can use delete to free the memory, since the method insert() dynamically allocate memories on the heap. As giblit mentioned in the example, if I have 10 people initially and remove 5 afterwards, then I have another 5 people which could cause memory leak when the class goes out of scope. So the desctructor uses delete to free the memory for the other 5 people. Am I correct?

But suppose another situation, if I remove all the 10 people, then there's nothing on the heap, can I still use delete in the destructor? Will this cause a problem? Or we should always use delete in the desctructor as a common practice?

Thank you.

[giblit]

The delete will not be called then in the destructor. When you insert you should increase the people by 1 and when you remove you should decrease by 1. If there are any left when the destructor is called it will delete 0-n people. You will not have any problems with deleting it twice because you should remove them from the vector after being removed with remove function.


The remove function should look something like:

bool remove(string nameToRemove)
{
    //find index of the name
    delete vec[index];
    vec.erase( vec.begin() + index );
    --numPeople;
    return success;
}

http://www.cplusplus.com/reference/vector/vector/erase/

I would suggest another stl container though because of the way vectors store things it has to reallocate memory if you remove anything but the last element.

[cire]

I would suggest another stl container though because of the way vectors store things it has to reallocate memory if you remove anything but the last element.

No it doesn't. It has to move/copy elements beyond the erased one. There is no reason for reallocation to occur in the vector.

[giblit]

Ah yeah sorry I missread thought it said reallocate but it said relocate.

erasing elements in positions other than the vector end causes the container to relocate all the elements after the segment erased to their new positions.

[ne555]

> vector<Person*> m_vec; // vector of pointers
justify the use of that instead of simply vector<Person> m_vec;


Note that your copy constructor and assignment operator are incorrect, leading to a double delete.
Maybe you should read Aho instead.


> Or we should always use delete in the desctructor as a common practice?
¿what are you going to delete?

[TwilightSpectre]

The use of a vector of pointers could be because the class is polymorphic, "Person" is certainly generic enough to be possible. However, I am not saying that you should use that. Rather, prefer to use either boost::ptr_container or std::vector<std::unique_ptr<Person>>.

[ne555]

> The use of a vector of pointers could be because the class is polymorphic,
void insert(string last, string first, size_t age); // insert one person into the array
¿so it is passing those three parameters to a factory?

[qmzh85]

Thanks all, appreciate all your explanations.