Annoying class hierarchy with mixed attributes

Question

I've created ColorBlock and ImageBlock classes subclassing from abstract Block class. Block implements size and position and has an abstract method draw(). ColorBlock implements color attribute and draws itself as a color square. ImageBlock implements image attribute and draws itself as a square with an image inside it.

Now I want to make the blocks movable, but I also want to keep the unmovable blocks. Basically I would have to create MovableColorBlock and MovableImageBlock, and they both implement the exact same moving action. The class hierarchy would be like this:

Block
|--ColorBlock
|----MovableColorBlock
|--ImageBlock
|----MovableImageBlock

And as you see, I'm implementing the moving action twice. An other way to implement this would be something like:

Block
|--ColorBlock
|--ImageBlock
|--MovableBlock (abstract)
|----MovableColorBlock
|----MovableImageBlock

Now I'm implementing the moving only once, but Color and Image twice. Is there an easy solution, or will I need duplicate code?

Answer 1

This is a problem that pretty much every graphics/physics engine experiences.

I would recommend having Movable block be a subclass of Block and having Color and Image block derive from that. After all, a non-moving block is just a moving block with no velocity.

Answer 2

You may want to look at the Decorator Pattern

For your particular case, you could imagine MovableBlock as a Decorator to Block, which could be applied to ColorBlock and ImageBlock (which would be ConcreteComponents to the Component Block).

Here's an alternate example - look at the ScrollableWindow description, which might make it easier to see the analogy with your example.

Answer 3

Another option is to create a Movable interface (pure virtual class) that declares methods for moving, and derive subclasses that implement those methods:

Block                 Movable
  |                     |
  +-ColorBlock----------+-MovableColorBlock
  |                     |
  +-ImageBlock----------+-MovableImageBlock

Edit

Yeah, if the move() method really is the same between the subclasses, then this isn't what you want.

It sounds like the move() operation is orthagonal to the draw() operation, which to me suggests an aggregation instead of a subclass. So let's take a slight left turn here.

A Block has a position; perhaps the positional parameter could be what's movable or not:

class Position 
{
  public:

  Position( int x, int y ) : m_x(x), m_y(y) { }
  virtual ~Position() {}

  int getX() const { return m_x; }
  int getY() const { return m_y; }

  virtual bool movable() const { return false; }

  protected:

  int m_x;
  int m_y;
};

class MovablePosition : public Position
{
  public:

  MovablePosition( int x, int y ) : Position( x, y ) {}
  virtual ~MovablePosition() {}

  void move( int newX, int newY ) { m_x = newX; m_y = newY; }

  virtual bool movable() const { return true; }
};

Then your base Block class takes a Position as a parameter:

class Block 
{
  public: 

  Block( Position *p ) : m_pos( p ) {}
  virtual ~Block() {}

  virtual void draw() = 0;

  Position *getPos() const { return m_pos; }

  protected:

  Position *m_pos;
};

Then if you want to move a Block subclass instance, you'd first check the movable() method:

if ( myColorBlock.getPos()->movable() )
{
  MovablePosition *p = myColorBlock.getPos();
  p->move( newX, newY );
  myColorBlock.draw();
}

No need to create redundant MovableXxxBlock classes, no repetition of code. Under this scheme, a block's movability is established at runtime, not compile time; that may or may not be compatible with your goals.

Answer 4

You can make your MovableColorBlock and MovableImageBlock friend classes of 'MoveableBlock' and inherit from their respective "non-moving" Block classes. You'll still have the two extra classes but the friend declaration will let them access the moving implementation in MoveableBlock