views:

173

answers:

2

Hello,

I'm working on a graphics application that is using virtual classes fairly extensively. It has:

  • A picture class, which is essentially a collection of shapes.

  • A shapes class, which is purely virtual and has a few classes that inherit from it:

    • Circle
    • Polygon
    • Rectangle
  • A Figure shape, which is any graphical figure (also virtual), shape inherits from this.

Essentially, my problem comes down to implementing the picture class, which is basically being used to store a collection of shapes. I'm currently using a Vector to store shapes, however, it's apparent that this is the wrong decision since Vector instantiates these shapes, which is not good as they are purely virtual.

Below is my current code base (summarized a bit):

class Figure {
public:
...
  virtual ~Figure();
...
};

class Shape: public Figure
{
public:
...
  virtual ~Shape() {}
  virtual Shape* clone() const = 0;
...
};

class Polygon : public Shape
{
public:
...
virtual Shape* clone() const {return new Polygon(*this);}
... 
private:
std::vector<Point> points;

};

class Picture: public Figure {
public:
...
  Picture(Graphics& gd);
  Picture (const Picture&);
  ~Picture();
  void clear();
  void add (const Shape&);
...
private:
std::vector<Shape> shapes;
Graphics* gfx;
};

//Picture implementation:
...
Picture::Picture(Graphics& gd)
{
gfx = &gd;
}


Picture::Picture(const Picture& a)
{
shapes = a.shapes;
}

Picture::~Picture() 
{
clear();
}

void Picture::clear()
{
shapes.clear();
}

void Picture::add (const Shape& shp)
{
Shape* nshp = shp.clone();
shapes.push_back(*nshp);
}
...

The error messages I'm getting are just a bunch of these:

picture.cpp:33: instantiated from here /opt/local/bin/../lib/gcc/sparc-sun-solaris2.10/4.4.1/../../../../include/c++/4.4.1/ext/new_allocator.h:105: error: cannot allocate an object of abstract type 'Shape' shape.h:12: note: because the following virtual functions are pure within 'Shape': shape.h:58: note: virtual void Shape::get(std::istream&) shape.h:31: note: virtual void Shape::put(std::ostream&) const shape.h:36: note: virtual void Shape::scale(const Point&, double) shape.h:40: note: virtual void Shape::translate(double, double) shape.h:45: note: virtual void Shape::reflectHorizontally(double) shape.h:49: note: virtual void Shape::reflectVertically(double) shape.h:52: note: virtual RectangularArea Shape::boundingBox() const shape.h:21: note: virtual Shape* Shape::clone() const shape.h:55: note: virtual void Shape::draw(Graphics&) const

So what is the ideal way to store these shapes. What kind of collection should I be using to store these things?

Thanks

+5  A: 

When you need polymorphism, you need to use either pointers or references. Since containers (or arrays) can't store references, you have to use pointers.

Essentially change your picture class's vector to:

std::vector<Shape*>

and appropriately modify the other member functions.

The reason why you can't/shouldn't store them as value types is because vector is a homogenous container i.e. it only stores data of one type (and only one type -- subclasses are not allowed!). The reason for this is because the vector stores its data in an array, which needs to know the size of the objects it's storing. If the sizes of these objects are different (which they might be for different shapes) then it can't store them in an array.

If you store them as pointers then they all have the same size (sizeof(Shape*)) and also have access to the shape's vtable, which is what allows polymorphic behaviour.

Peter Alexander
With C++0x features like improved smart pointers and move semantics, one could even make a memory-safe version of this (`vector<unique_ptr<Shape> >`).
bobbymcr
If you don't use smart pointers, remember the container won't automatically clean up the pointed-to data. It calls destructors on the pointers - but those destructors don't do anything. You should add a destructor to your picture class which iterates through the shapes deleting them. Using a smart pointer class, as bobbymcr suggests, avoids this issue.
Steve314
You are the man! Thank you so much. That was it.
the_gastropod
+1  A: 

Use covariant return types. See FAQ 20.8 for your clone methods. You can rely on the factory method as well to create the Shape objects.

Also, you cannot have a container of abstract class objects, abstract classes cannot be instantiated. Instead, create a container of pointers/references to derived concrete objects. Note, if you are using pointer, it becomes your responsibility to clear them. The container will not de-allocate the memory properly. You can use smart pointers instead of raw pointers to handle this more efficiently. Look up scoped_ptr and shared_ptr from Boost.

dirkgently
On first skim, I saw the "covariant return types" and assumed it was answering the wrong question. Putting that after the main answer may have earned you more upvotes. You got my +1 now though.
Steve314