CODEWITHSUNDEEP
c++templatesdelete-operator
Roy Gavrielov
Roy Gavrielov

Reputation: 607

How to delete a template?

I'm having trouble with deleting my template.
My template and destructor: 

template<class S, class T>  
class Consortium
{

private :

    map<const S, Node<T>*> m_consortiumMap;
    Heap<T>m_consortiumHeap;

public :

    ~Consortium();
    void Insert(const S key, T toAdd);
    void Update(const S key);
    void Remove(const S key);
    const T Top();
};

template<class S, class T>
Consortium<S,T>::~Consortium()
{
    m_consortiumMap.clear();
    delete &m_consortiumHeap.;
}

My heap and destructor: 

template <class T>
class Heap
{
private :

    vector<Node<T>*> m_heapVector;

public :

    ~Heap();

    int parent(int i) const {return i / 2;}
    int left(int i) const {return 2 * i;}
    int right(int i) const {return 2 * i + 1;}
    void heapify(int index);
    Node<T>* extractMin ();
    void heapDecreaseKey (int index, Node<T>* key);
    void MinHeapInsert (Node<T>* key);
    Node<T>* ExtractNode(int index);
    Node<T>* top ()const {return m_heapVector[0];}

};  

template<class T>
Heap<T>::~Heap()
{
    for (int i = 0 ; i < m_heapVector.size() ; i++)
        m_heapVector.erase(m_heapVector.begin() + i);
}

and this is the object that holds the template, I'm having problems with that also:

class Garage
{
    private :

        Consortium<string, Vehicle*> m_consortium;

    public :

        ~Garage() {delete &m_consortium;}
};

what's wrong here?

Upvotes: 0

Views: 4771

Answers (4)

Ferruccio
Ferruccio

Reputation: 100718

If you didn't use new to create an object you can't use delete to get rid of it.

Upvotes: 2

Tyler McHenry
Tyler McHenry

Reputation: 76710

This is wrong on its face:

delete &m_consortiumHeap;

You must only delete things that you allocated with new. m_consortiumHeap is part of the class and gets automatically allocated when the class gets allocated and automatically deallocated when the class gets deallocated. You cannot and must not explicitly delete it.

This may have the opposite problem:

m_consortiumMap.clear();

the contents of m_consortiumMap are pointers. I can't tell from the code you've shown, but if the nodes within the map are allocated by the Consortium class using new, they must be deleteed, otherwise you will leak memory. Clearing the map will only get rid of the pointers, it will not deallocate the memory that they point to. You must first iterate through the map and delete each element. While deallocation of the elements is important, clearing the map in the destructor is kind of pointless since the map itself will be destroyed immediately afterwards anyway.

This is just perplexing:

for (int i = 0 ; i < m_heapVector.size() ; i++)
    m_heapVector.erase(m_heapVector.begin() + i);

first of all, everything I said about m_consortiumMap applies also to m_heapVector: If the contents were allocated with new by the Heap class, you must delete them in the destructor. And erasing the pointers from the vector is pointless, not to mention that the above loop has a logic error in it. When you iterate over a container, you should use the iterators themselves, e.g.

for (std::vector<Node<T>*>::iterator i = m_heapVector.begin() ; i != m_heapVector.end() ; i++)

Also, std::vector, like std::map, has a clear() function, but like I said it's pointless to clear the vector in the destructor. What you really want to do is deallocate the elements (if necessary).

Upvotes: 2

Patrick
Patrick

Reputation: 23629

Since m_consortiumHeap is a data member of your class (directly, not a pointer to it), you don't have to explicitly delete it. When the Consortium instance is destructed, it will automatically call the destructor of m_consortiumHeap for you.

Upvotes: 0

Cătălin Pitiș
Cătălin Pitiș

Reputation: 14341

You probably want to delete the objects pointed by the elements in the vector. Erase method doesn't do that, it just remove the pointer element from the vector, without destroying the pointed object. So you need (I presume) to delete the pointed object first, to avoid memory leaks. You cold do this:

for( vector<Node<T>*>::iterator iter = m_heapVector.begin(), endI = m_heapVector.end(); iter != endI; ++iter)
{
   delete *iter;
}

// m_heapVector.clean(); // Not necessary in destructor, since the vector will be destroyed anyway.

Using C++0x functions:

std::for_each( m_heapVector.begin(), m_heapVector.end(), []( Node<T>* node) { delete node; });

Also, use clear() method of the container (vector in your case) to remove all the elements.

Upvotes: 1

Related Questions