Should I change my design to prevent dynamic casts?

Question

I have read several threads about dynamic casts in C++, all full of people claiming it indicates bad design. In other languages I never gave it much thought when checking the type of an object. I never use it as an alternative to polymorphism and only when strong coupling seems perfectly acceptable. One of these situations i encounter quite often: having a list (i use std::vector in C++) of objects, all derived from a common base class. The list is managed by an object that is allowed to know the different subclasses (often it's a small hierarchy of private classes within the managing objects class). By keeping them in a single list (array, vector, ..) I can still benefit from polymorphism, but when an operation is meant to act on objects of a specific subclass I use a dynamic cast or something similar.

Is there a different approach to this type of problem without dynamic casts or type checking that I am missing? I am really curious how programmers that avoid these at all costs would handle them.

If my description is too abstract I could write a simple example in C++ (Edit: see below).

class EntityContacts {
private:
  class EntityContact {
  private:
    virtual void someVirtualFunction() { };            // Only there to make dynamic_cast work
  public:
      b2Contact* m_contactData;
  };

  class InternalEntityContact : public EntityContact {
  public:
    InternalEntityContact(b2Fixture* fixture1, b2Fixture* fixture2){
        m_internalFixture1 = fixture1;
        m_internalFixture2 = fixture2;
    };

    b2Fixture* m_internalFixture1;
    b2Fixture* m_internalFixture2;
  };

  class ExternalEntityContact : public EntityContact {
  public:
    ExternalEntityContact(b2Fixture* internalFixture, b2Fixture* externalFixture){
        m_internalFixture = internalFixture;
        m_externalFixture = externalFixture;
    };

    b2Fixture* m_internalFixture;
    b2Fixture* m_externalFixture;
  };

  PhysicsEntity* m_entity;
  std::vector<EntityContact*> m_contacts;
public:
  EntityContacts(PhysicsEntity* entity)
  {
    m_entity = entity;
  }

  void addContact(b2Contact* contactData)
  {
    // Create object for internal or external contact
    EntityContact* newContact;
    if (m_entity->isExternalContact(contactData)) {
        b2Fixture* iFixture;
        b2Fixture* eFixture;
        m_entity->getContactInExFixtures(contactData, iFixture, eFixture);
        newContact = new ExternalEntityContact(iFixture, eFixture);
    }
    else
        newContact = new InternalEntityContact(contactData->GetFixtureA(), contactData->GetFixtureB());

    // Add object to vector
    m_contacts.push_back(newContact);
  };

  int getExternalEntityContactCount(PhysicsEntity* entity)
  {
    // Return number of external contacts with the entity
    int result = 0;
    for (int i = 0; i < m_contacts.size(); ++i) {
        ExternalEntityContact* externalContact = dynamic_cast<ExternalEntityContact*>(m_contacts[i]);
        if (externalContact != NULL && getFixtureEntity(externalContact->m_externalFixture) == entity)
            result++;
    }
    return result;
  }
};

It is a simplified version of a class that i use for collision detection in a game that uses box2d physics. I hope that the box2d details don't distract too much from what i am trying to show. I have a very similar class 'Event' that creates different types of event handlers which is structured in the same way (with subclasses of a base class EventHandler instead of EntityContact).

Answer 1

At least from my perspective, dynamic_cast exists for a reason, and there are times it's reasonable to use it. This may be one of those times.

Given the situation you describe, one possible alternative may be to define more of the operations you need in the base class, but define them as (possibly silently) failing if you invoke them for the base class or other classes that don't support those operations.

The real question is whether it makes sense to define your operations this way. Going back to the typical animal-based hierarchy, if you're working with Birds, it's often sensible for the Bird class to define an fly member, and for the few birds that can't fly, just have it fail (theoretically should be renamed as something like attempt_to_fly, but that rarely accomplishes much).

If you're seeing a lot of this, it tends to indicate a lack of abstraction in your classes -- for example, instead of a fly or attempt_to_fly, you might really want a travel member, and it's up to the individual animal to determine whether to do that by swimming, crawling, walking, flying, etc.