views:

128

answers:

5
+1  A: 

You could add a clone method to your interface to, well, clone instances of your type hierarchy. Each concrete implementation produces a new instance of that specific class, so all type information is preserved.

class Object {
public:
  virtual Object clone() = 0;
};

class Rectangle : public Object {
public:
  virtual Rectangle clone() { /* create and return an identical Rectangle */ }
};

class Ellipse : public Object {
public:
  virtual Ellipse clone() { /* create and return an identical Ellipse */ }
};

class Plane : public Rectangle {
  std::vector<Object*> plane_objects;
public:
  virtual Plane clone() { /* create and return an identical Plane */ }
  void AddObject(const Object& object) {
    plane_objects.push_back(object.clone());
  }
};

This implies that the plane is the owner of all its objects, so it should also destroy them in its destructor. If those objects are accessed only internally, they can be stored with plain pointers - although even then smart pointers make disposal simpler and safer. If they are published, however, it is better to use smart pointers to store them.

Péter Török
+3  A: 

Use a smart pointer like boost::shared_ptr or boost::intrusive_ptr.

Why do you assume there is something better than using a smart pointer? Storing raw pointers in a container generally ends in tears if you don't take extraordinary steps to ensure exception safety in your code.

James McNellis
But this doesn't actually answer the question posed, which is one of polymorphic cloning.
dash-tom-bang
@dash: The original question was about ownership maintenance, which is what this answers.
James McNellis
+1  A: 

when destroying the plane? why? are we sure that calls to AddObject were only done as AddObject(new something).

It's impossible to be sure. But something has to manage the object lifetimes. You should simply clearly document which thing that is.

anon
+3  A: 

Your actual problem seems to be managing the objects' lifetimes. Four possibilities that come to mind are:

  1. Your container (i.e. Plane) assumes ownership of all contained objects and therefore deletes them once it's itself destroyed.

  2. Your container (Plane) does not assume ownership and whoever added objects to your container will be responsible for destroying them.

  3. The lifetime of your objects is managed automatically.

  4. You circumvent the problem by providing the container with a clone of the actual object. The container manages a copy of the object, and the caller manages the original object.

What you currently have seems like approach #4. By doing:

plane_objects.push_back(new Object(object));

you insert a copy of the object into the container. Therefore the problem sort of disappears. Ask yourself whether this is really what you want, or if one of the above choices would be more suitable.


Options #1 and #2 are easy to implement, because they define a contract that your implementation simply has to follow. Option #3 would call for e.g. smart pointers, or some other solution involving reference counting.

If you want to keep following the approach of option #4, you could e.g. extend your Object class with a clone method, so that it returns the right type of object. This would get rid of the incorrect new Object(...).

class Object
{
    public:
        virtual Object* clone() const = 0;
        ...
};

...

Object* Rectangle::clone() const
{
    return new Rectangle(*this);  // e.g. use copy c'tor to return a clone
}

P.S.: Note how the STL containers seem to deal with this issue: Let's say you declare a vector<Foo>. This vector will contain copies of the objects inserted into it (option #4 in my answer). However, if you declare the collection as vector<Foo*>, it will contain references to the original objects, but it will not manage their lifetime (option #2 in my answer).

stakx
+1  A: 

First you need to figure out your object identity and ownership semantics:

Is each Object a real identity or does it simply refer to a real entity? For the former, you cannot use clone() and you need to use reference counting (via boost::shared_ptr) or pass by reference if the owner of the instance is guaranteed to outlive the references to it. For the latter, clone() might make more sense and you can safely ignore ownership issues.

In general you should avoid passing naked pointers around and use boost::shared_ptr instead or references.

Chris Hafey