Template for cleaning up on vector destruction - how to?

Question

I am trying to implement a template which would allow to create a class derived from vector<> such that on deletion the element of the vector are deleted. The below snippet represents an attempt to do so:

include <vector>

using namespace std;

template <class P>
class TidyVector : public vector<P> {
 public:
  ~TidyVector() {
    while (vector<P>::size()) {
      P pi = vector<P>::back();
      vector<P>::pop_back();
      delete pi;
    }
  }
}

TidyVector<int*> i;

Attempts to compile this using g++ -c try.cc result in the following error messages:

try.cc:1: error: expected constructor, destructor, or type conversion before '<' token
try.cc:6: error: expected template-name before '<' token
try.cc:6: error: expected `{' before '<' token
try.cc:6: error: expected unqualified-id before '<' token
try.cc:17: error: aggregate 'TidyVector<int*> i' has incomplete type and cannot be defined

What is going on - why does this not work? Or, maybe, a more appropriate question to ask: what is the standard way to deal with this situation (automated clean up of the vector on deletion)?

Answer 1

I assume that is supposed to be #include < vector >. So that's one bug.

Second, you should avoid inheriting from vector, it doesn't have a virtual destructor, so your class could get easily cast to a standard vector and your destructor not run.

(oh, and you left the semi colon off the class end)

Third, have you considered just doing vector< boost::shared_ptr< some_class > > instead?

Clarifying a bit on why shared_ptr approach is better: Your class right now only cleans things up in one instance; if they are still there when the class is destructed. But say you have a TidyVector foo with 10 elements and then someone does:

foo[5] = new int; // Memory leak!

Or

foo.resize(0); // Memory leak!

Or

foo.erase(foo.begin()); // Memory leak!

The list goes on

Answer 2

Read the compiler error messages: You're missing a '#' from #include, for starters.

In the your destructor, you don't need to specify vector<P>:: - you are deriving from the vector, so call its methods without qualification, e.g. size() etc.

The vector will clean up its own storage so you don't need to call pop_back() on it. Try

for (iterator i = begin(); i != end; ++i)
    delete *i;

Instead of doing this manually, you cold hold a smart pointer such as shared_ptr in your vector, the elements would then be deleted for you without needing a derived class.

There are differing opinions on whether deriving from standard library containers is a good idea, a search here will point you to several discussions on the issue.

Answer 3

There is already a class that does this: Boost's ptr_vector. I suggest checking it out, and looking at how they did it. Short answer: they don't use inheritance, they use composition.