0

I created a container that controls the life cycle (new/delete) of certain types of objects to avoid any programming mistakes. For example, a object is deleted without any notification to the container. The objects inherit from the same base class (GreetingBase).

For the implementation I am using a "template trick":

class GreetingBase{
public:
  virtual void sayHello(){
    std::cout << "Hello there!" << endl;
  }

  virtual ~GreetingBase(){}
};

class GreetingDerived: public GreetingBase{
public:
  virtual void sayHello(){
    std::cout << "Hola amigos!" << endl;
  }
  virtual ~GreetingDerived(){}
};

class GreetingContainer{
public:
   template <class T>
   void addClass(){
      items.push_back(new T());
   }
~GreetingContainer(){
   for(std::vector<GreetingBase*>::iterator it = items.begin();
   it < items.end(); it++ ){
   delete *it;
 }
}

void talk(){
  for(std::vector<GreetingBase*>::iterator it = items.begin();
    it < items.end(); it++ ){
      (*it)->sayHello();
   }
  }
private:
 std::vector<GreetingBase*> items;
};


int main(){
  GreetingContainer container;
  container.addClass<GreetingDerived>();
  container.talk();
  return 0;
}

Questions:

  1. Using templates in order to solve this problem is a common approach? any drawbacks?
  2. Any "standard way" to report better errors messages when "T" is not derived from "GreetingBase"

Thank you in advance.

Víctor Herraiz
  • 1,132
  • 1
  • 11
  • 26

3 Answers3

2

Using templates in order to solve this problem is a common approach?

I don't know whether it's common, but it looks sensible enough. It ensures that your container can only contain pointers to objects allocated with new, which is good.

any drawbacks?

The main problem is that your container breaks the Rule of Three. It has an implicitly generated copy constructor and copy-assignment operator which simply copy each pointer; that will give you two container objects whose destructors both try to delete the same objects.

The easiest way to fix that is by deleting these member functions, or declaring them private (with no implementation) if you're stuck with a pre-2011 version of the language. If you need to be able to copy the container, then you'll need to implement them to do so safely.

Personally, I'd use smart pointers rather than rolling my own RAII container; std::unique_ptr if the container is to have exclusive ownership, std::shared_ptr if you want to share ownership, or perhaps std::weak_ptr to hold non-owning references to objects managed elsewhere by shared pointers. If you're stuck in the past, then unique_ptr won't be available, but Boost provides shared_ptr, weak_ptr, and also Pointer containers similar to yours.

Any "standard way" to report better errors messages when "T" is not derived from "GreetingBase"

In C++11, you could perhaps use a static assert, something like:

static_assert(std::is_base_of<GreetingBase, T>::value, 
              "Wrong type for GreetingContainer");

Alternatively, you might get a slightly more readable error message by creating a local pointer; then the error message at least won't contain the full name of push_back:

GreetingBase * p = new T();
items.push_back(p);

The error message will be something like can't convert Whatever* to GreetingBase*, which should be clear enough.

Community
  • 1
  • 1
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
  • Thanks. I am trying to avoid anyone could delete the instance from outside the container. any suggestions? – Víctor Herraiz Sep 25 '12 at 10:37
  • @VíctorHerraiz: As your code stands, no-one can access the pointers directly anyway, so that's not an issue. If you add accessors, have them return references or smart pointers, rather than raw pointers, so no-one will incorrectly think that they should delete them. You could perhaps make the base-class destructor protected and declare the container a friend, if you really wanted to prevent people from deliberately deleting things that they know they shouldn't; but I wouldn't do that, since it limits the usability of the classes, forcing you to use the container to manage them. – Mike Seymour Sep 25 '12 at 10:47
1
  1. it's ok to use template for this, but use function, that receives pointer to base-class is better.
  2. standard C++11 way - use static_assert with std::is_base_of. Without C++11 - boost equivalents, or your own meta-functions.

Side note: if you use smart pointers, there is no need to write such containers

ForEveR
  • 55,233
  • 2
  • 119
  • 133
0

I see no advantage in making addClass a template; it just means that you get a new copy of the code instantiated for every different type, and you're enforcing a particular constructor call.

Just make it a plain function taking a base class pointer:

void addClass(GreetingBase *o){
    items.push_back(o);
}

So your callers do

container.addClass( new GreetingDerived() );

instead of

container.addClass<GreetingDerived>();
Frerich Raabe
  • 90,689
  • 19
  • 115
  • 207