views:

214

answers:

3

Let's say we already have a hierarchy of classes, e.g.

class Shape { virtual void get_area() = 0; };
class Square : Shape { ... };
class Circle : Shape { ... };
etc.

Now let's say that I want to (effectively) add a virtual draw() = 0 method to Shape with appropriate definitions in each sub-class. However, let's say I want to do this without modifying those classes (as they are part of a library that I don't want to change).

What would be the best way to go about this?

Whether or not I actually "add" a virtual method or not is not important, I just want polymorphic behaviour given an array of pointers.

My first thought would be to do this:

class IDrawable { virtual void draw() = 0; };
class DrawableSquare : Square, IDrawable { void draw() { ... } };
class DrawableCircle : Circle, IDrawable { void draw() { ... } };

and then just replace all creations of Squares and Circles with DrawableSquares and DrawableCircles, respectively.

Is that the best way to accomplish this, or is there something better (preferably something that leaves the creation of Squares and Circles intact).

Thanks in advance.

+3  A: 

What you're describing is somewhat like the decorator pattern. Which is very suitable to change runtime behaviour of existing classes.

But I don't really see how to implement your practical example, if shapes have no way to be drawn, then there's no way to change drawing behaviour at runtime either...

But I suppose this is just a very simplified example for stackoverflow? If all the basic building blocks for the desired functionality are available, then implementing the exact runtime behaviour with such a pattern is certainly a decent option.

Pieter
This isn't a simplified example for SO. I literally have shapes that I want to add drawing methods to. Specifically, the shapes are from the Bullet Physics SDK collision shapes and I want to be able to add drawing functionality for debugging purposes.
Peter Alexander
Correct me if I'm wrong, but I don't think decorator solves this problem. Decorator adds a new tree of behaviour to existing classes, rather than adding behaviour to the existing tree.
Peter Alexander
It looks like the Bullet SDK has a `btIDebugDraw` interface: http://bulletphysics.com/Bullet/BulletFull/classbtIDebugDraw.html . You derive a class from it that implements functions such as `drawBox()` and `drawSphere()`, then assign it to a `btCollisionWorld` or `btDynamicsWorld`, and then have it draw that world. Would that do what you needed?
Josh Townzen
+4  A: 

(I do propose a solution down further... bear with me...)

One way to (almost) solve your problem is to use a Visitor design pattern. Something like this:

class DrawVisitor
{
public:
  void draw(const Shape &shape); // dispatches to correct private method
private:
  void visitSquare(const Square &square);
  void visitCircle(const Circle &circle);
};

Then instead of this:

Shape &shape = getShape(); // returns some Shape subclass
shape.draw(); // virtual method

You would do:

DrawVisitor dv;
Shape &shape = getShape();
dv.draw(shape);

Normally in a Visitor pattern you would implement the draw method like this:

DrawVisitor::draw(const Shape &shape)
{
  shape.accept(*this);
}

But that only works if the Shape hierarchy was designed to be visited: each subclass implements the virtual method accept by calling the appropriate visitXxxx method on the Visitor. Most likely it was not designed for that.

Without being able to modify the class hierarchy to add a virtual accept method to Shape (and all subclasses), you need some other way to dispatch to the correct draw method. One naieve approach is this:

DrawVisitor::draw(const Shape &shape)
{
  if (const Square *pSquare = dynamic_cast<const Square *>(&shape))
  {
    visitSquare(*pSquare);
  }
  else if (const Circle *pCircle = dynamic_cast<const Circle *>(&shape))
  {
    visitCircle(*pCircle);
  }
  // etc.
}

That will work, but there is a performance hit to using dynamic_cast that way. If you can afford that hit, it is a straightforward approach that is easy to understand, debug, maintain, etc.

Suppose there was an enumeration of all shape types:

enum ShapeId { SQUARE, CIRCLE, ... };

and there was a virtual method ShapeId Shape::getId() const = 0; that each subclass would override to return its ShapeId. Then you could do your dispatch using a massive switch statement instead of the if-elsif-elsif of dynamic_casts. Or perhaps instead of a switch use a hashtable. The best case scenario is to put this mapping function in one place, so that you can define multiple visitors without having to repeat the mapping logic each time.

So you probably don't have a getid() method either. Too bad. What's another way to get an ID that is unique for each type of object? RTTI. This is not necessarily elegant or foolproof, but you can create a hashtable of type_info pointers. You can build this hashtable in some initialization code or build it dynamically (or both).

DrawVisitor::init() // static method or ctor
{
  typeMap_[&typeid(Square)] = &visitSquare;
  typeMap_[&typeid(Circle)] = &visitCircle;
  // etc.
}

DrawVisitor::draw(const Shape &shape)
{
  type_info *ti = typeid(shape);
  typedef void (DrawVisitor::*VisitFun)(const Shape &shape);
  VisitFun visit = 0; // or default draw method?
  TypeMap::iterator iter = typeMap_.find(ti);
  if (iter != typeMap_.end())
  {
    visit = iter->second;
  }
  else if (const Square *pSquare = dynamic_cast<const Square *>(&shape))
  {
    visit = typeMap_[ti] = &visitSquare;
  }
  else if (const Circle *pCircle = dynamic_cast<const Circle *>(&shape))
  {
    visit = typeMap_[ti] = &visitCircle;
  }
  // etc.

  if (visit)
  {
    // will have to do static_cast<> inside the function
    ((*this).*(visit))(shape);
  }
}

Might be some bugs/syntax errors in there, I haven't tried compiling this example. I have done something like this before -- the technique works. I'm not sure if you might run into problems with shared libraries though.

One last thing I'll add: regardless of how you decide to do the dispatch, it probably makes sense to make a visitor base class:

class ShapeVisitor
{
public:
  void visit(const Shape &shape); // not virtual
private:
  virtual void visitSquare(const Square &square) = 0;
  virtual void visitCircle(const Circle &circle) = 0;
};
Dan
Philip Potter
@Philip: oops... fixed.
Dan
Interesting solution, I like the use of only part of the `Visitor` pattern >> patterns are meant to be adapted to the situation, and not the other way around :)
Matthieu M.
A: 

One 'off the wall' solution you might like to consider, depending on the circumstance, is to use templates to give you compile time polymorphic behaviour. Before you say anything, I know that this will not give you traditional runtime polymorphism so it may well not be useful but depending on the limitations of the environment in which you're working, it can prove useful:

#include <iostream>

using namespace std;

// This bit's a bit like your library.
struct Square{};
struct Circle{};
struct AShape{};

// and this is your extra stuff.
template < class T >
class Drawable { public: void draw() const { cout << "General Shape" << endl; } };

template <> void Drawable< Square >::draw() const { cout << "Square!" << endl; };
template <> void Drawable< Circle >::draw() const { cout << "Circle!" << endl; };

template < class T >
void drawIt( const T& obj )
{
  obj.draw();
}

int main( int argc, char* argv[] )
{
  Drawable<Square> a;
  Drawable<Circle> b;
  Drawable<AShape> c;

  a.draw(); // prints "Square!"
  b.draw(); // prints "Circle!"
  c.draw(); // prints "General Shape" as there's no specific specialisation for an Drawable< AShape >

  drawIt(a); // prints "Square!"
  drawIt(b); // prints "Circle!"
  drawIt(c); // prints "General Shape" as there's no specific specialisation for an Drawable< AShape >
}

The drawIt() method is probably the key thing here as it represents generic behaviour for any class meeting the requirement of having a draw() method. Do watch out for code bloat here though as the compiler will instantiate a separate method for each type passed.

This can be useful in situations where you need to write one function to work on many types which have no common base class. I'm aware that this is not the question you asked, but I thought I'd throw it just as an alternative.

Robin Welch