Use case of dynamic_cast

Question

In many places you can read that dynamic_cast means "bad design". But I cannot find any article with appropriate usage (showing good design, not just "how to use").

I'm writing a board game with a board and many different types of cards described with many attributes (some cards can be put on the board). So I decided to break it down to the following classes/interfaces:

class Card {};
class BoardCard : public Card {};
class ActionCard : public Card {};
// Other types of cards - but two are enough
class Deck {
    Card* draw_card();
};
class Player {
    void add_card(Card* card);
    Card const* get_card();
};
class Board {
    void put_card(BoardCard const*);
};

Some guys suggested that I should use only one class describing a card. But I would mean many mutually excluding attributes. And in the case of the Board class' put_card(BoardCard const&) - it is a part of the interface that I cannot put any card on the board. If I had only one type of card I would have to check it inside the method.

I see the flow like the following:

• a generic card is in the deck (it's not important what its type is)
• a generic card is drawn from the deck and given to a player (the same as above)
• if a player chosen a BoardCard then it can be put on the board

So I use dynamic_cast before putting a card on the board. I think that using some virtual method is out of the question in this case (additionally I wouldn't make any sense to add some action about board to every card).

So my question is: What have I designed badly? How could I avoid dynamic_cast? Using some type attribute and ifs would be a better solution...?

P.S. Any source treating about dynamic_cast usage in the context of design is more than appreciated.

Answer 1

Yes, dynamic_cast is a code smell, but so is adding functions that try to make it look like you have a good polymorphic interface but are actually equal to a dynamic_cast i.e. stuff like can_put_on_board. I'd go as far as to say that can_put_on_board is worse - you're duplicating code otherwise implemented by dynamic_cast and cluttering the interface.

As with all code smells, they should make you wary and they don't necessarily mean that your code is bad. This all depends on what you're trying to achieve.

If you're implementing a board game that will have 5k lines of code, two categories of cards, then anything that works is fine. If you're designing something larger, extensible and possibly allowing for cards being created by non-programmers (whether it's an actual need or you're doing it for research) then this probably won't do.

Assuming the latter, let's look at some alternatives.

You could put the onus of applying the card properly to the card, instead of some external code. E.g. add a play(Context& c) function to the card (the Context being a means to access the board and whatever may be necessary). A board card would know that it may only be applied to a board and a cast would not be necessary.

I would entirely give up using inheritance however. One of its many issues is how it introduces a categorisation of all cards. Let me give you an example:

• you introduce BoardCard and ActionCard putting all cards in these two buckets;
• you then decide that you want to have a card that can be used in two ways, either as an Action or a Board card;
• let's say you solved the issue (through multiple-inheritance, a BoardActionCard type, or any different way);
• you then decide you want to have card colours (as in MtG) - how do you do this? Do you create RedBoardCard, BlueBoardCard, RedActionCard etc?

Other examples of why inheritance should be avoided and how to achieve runtime polymorphism otherwise you may want to watch Sean Parent's excellent "Inheritance is the Base Class of Evil" talk. A promising looking library that implements this sort of polymorphism is dyno, I have not tried it out yet though.

A possible solution might be:

class Card final {
public:
    template <class T>
    Card(T model) :
        model_(std::make_shared<Model<T>>(std::move(model)))
    {}

    void play(Context& c) const {
        model_->play(c);
    }

    // ... any other functions that can be performed on a card

private:

    class Context {
    public:
        virtual ~Context() = default;
        virtual void play(Context& c) const = 0;
    };

    template <class T>
    class Model : public Context {
    public:
        void play(Context& c) const override {
            play(model_, c);

            // or

            model_.play(c);

            // depending on what contract you want to have with implementers
        }
    private:
        T model_;
    };

    std::shared_ptr<const Context> model_;

};

Then you can either create classes per card type:

class Goblin final {
    void play(Context& c) const {
        // apply effects of card, e.g. take c.board() and put the card there
    }
};

Or implement behaviours for different categories, e.g. have a

template <class T>
void play(const T& card, Context& c);

template and then use enable_if to handle it for different categories:

template <class T, class = std::enable_if<IsBoardCard_v<T>>
void play(const T& card, Context& c) {
    c.board().add(Card(card));
}

where:

template <class T>
struct IsBoardCard {
    static constexpr auto value = T::IS_BOARD_CARD;
};

template <class T>
using IsBoardCard_v = IsBoardCard<T>::value;

then defining your Goblin as:

class Goblin final {
public:
    static constexpr auto IS_BOARD_CARD = true;
    static constexpr auto COLOR = Color::RED;
    static constexpr auto SUPERMAGIC = true;
};

which would allow you to categorise your cards in many dimensions also leaving the possibility to entirely specialise the behaviour by implementing a different play function.

The example code uses std::shared_ptr to store the model, but you can definitely do something smarter here. I like to use a static-sized storage and only allow Ts of a certain maximum size and alignment to be used. Alternatively you could use a std::unique_ptr (which would disable copying though) or a variant leveraging small-size optimisation.

Answer 2

Why not use dynamic_cast

dynamic_cast is generally disliked because it can be easily abused to completely break the abstractions used. And it is not wise to depend on specific implementations. Of course it may needed, but really rarely, so nearly everyone takes a rule of thumb - probably you should not use it. It's a code smell that may imply that you should rethink Your abstractions because they may be not the ones needed in Your domain. Maybe in Your game the Board should not have put_card method - maybe instead card should have method play(const PlaySpace *) where Board implements PlaySpace or something like that. Even CppCoreGuidelines discourage using dynamic_cast in most cases.

When use

Generally few people ever have problems like this but I came across it multiple times already. The problem is called Double (or Multiple) Dispatch. Here is pretty old, but quite relevant article about double dispatch (mind the prehistoric auto_ptr): http://www.drdobbs.com/double-dispatch-revisited/184405527

Also Scott Meyers in one of his books wrote something about building double dispatch matrix with dynamic_cast. But, all in all, these dynamic_casts are 'hidden` inside this matrix - users don't know what kind of magic happens inside.

Noteworthy - multiple dispatch is also considered code smell :-).

Reasonable alternative

Check out the visitor pattern. It can be used as replace for dynamic_cast but it is also some kind of code smell.

I generally recommend using dynamic_cast and visitor as a last resort tools for design problems as they break abstraction which increases complexity.

Answer 3

You could apply the principles behind Microsoft's COM and provide a series of interfaces, with each interface describing a set of related behaviors. In COM you determine if a specific interface is available by calling QueryInterface, but in modern C++ dynamic_cast works similarly and is more efficient.

class Card {
    virtual void ~Card() {} // must have at least one virtual method for dynamic_cast
};
struct IBoardCard {
    virtual void put_card(Board* board);
};
class BoardCard : public Card, public IBoardCard {};
class ActionCard : public Card {};
// Other types of cards - but two are enough
class Deck {
    Card* draw_card();
};
class Player {
    void add_card(Card* card);
    Card const* get_card();
};
class Board {
    void put_card(Card const* card) {
        const IBoardCard *p = dynamic_cast<const IBoardCard*>(card);
        if (p != null) p->put_card(this);
};

That may be a bad example, but I hope you get the idea.