C++ Design issues: Map with various abstract base classes

Question

I'm facing design problems and could do with some external input. I am trying to avoid abstract base class casting (Since I've heard that's bad).

The issues are down to this structure:

class entity... (base with pure virtual functions)
class hostile : public entity... (base with pure virtual functions)
class friendly : public entity... (base with pure virtual functions)
// Then further derived classes using these last base classes... 

Initially I thought I'd get away with:

const enum class FactionType : unsigned int
{ ... };

std::unordered_map<FactionType, std::vector<std::unique_ptr<CEntity>>> m_entitys;

And... I did but this causes me problems because I need to access "unique" function from say hostile or friendly specifically.

I have disgracefully tried (worked but don't like it nor does it feel safe):

// For-Each Loop: const auto& friendly : m_entitys[FactionType::FRIENDLY]
CFriendly* castFriendly = static_cast<CFriendly*>(&*friendly);

I was hoping/trying to maintain the unordered_map design that uses FactionType as a key for the base abstract class type... Anyway, input is greatly appreciated.

If there are any syntactical errors, I apologise.

Answer 1

About casting I agree with with @rufflewind. The casts mean different thing and are useful at different times.

To coerce a region of memory at compile time (the decision of typing happen at compile time anyway) use static_cast. The amount of memory on the other end of the T* equal to sizeof(T) will be interpreted as a T regardless of correct behavior.

The decisions for dynamic_cast are made entirely at runtime, sometimes requiring RTTI (Run Time Type Information). It makes a decision and it will either return a null pointer or a valid pointer to a T if one can be made.

---

The decision goes further than just the types of casts though. Using a data structure to look up types and methods (member functions) imposes the time constraints that would not otherwise exist when compared to the relatively fast and mandatory casts. There is a way to skip the data structures, but not the casting without major refactoring (with major refactoring you can do anything).

You can move the casts into the entity class, get them done right and just leave them encapsulated there.

class entity
{
    // Previous code
    public:

    // This will be overridden in hostiles to return a valid 
    // pointer and nullptr or 0 in other types of entities
    virtual hostile* cast_to_hostile() = 0
    virtual const hostile* cast_to_hostile() const = 0

    // This will be overridden in friendlies to return a valid 
    // pointer and nullptr or 0 in other types of entities
    virtual friendly* cast_to_friendly() = 0
    virtual const friendly* cast_to_friendly() const = 0


    // The following helper methods are optional but
    // can make it easier to write streamlined code in
    // calling classes with a little trouble.

    // Hostile and friendly can each implement this to return
    // The appropriate enum member. This would useful for making
    // decision about friendlies and hostiles
    virtual FactionType entity_type() const = 0;

    // These two method delegate storage of the knowledge
    // of hostility or friendliness to the derived classes.
    // These are implemented here as non-virtual functions
    // because they shouldn't need to be overridden, but 
    // could be made virtual at the cost of a pointer
    // indirection and sometimes, but not often a cache miss.
    bool is_friendly() const
    {
        return entity_type() == FactionType_friendly;
    }
    bool is_hostile() const
    {
        return entity_type() == FactionType_hostile;
    }
}

This strategy is good and bad for a variety of reasons.

Pros:

It is conceptually simple. This is easy to understand quickly if you understand polymorphism.

It seems similar to your existing code seems superficially similar to your existing code making migration easier. There is a reason hostility and friendliness is encoded in your types, this preserves that reason.

You can use static_casts safely because all the casts exist in the class they are used in, and therefor won't normally get called unless valid.

You can return shared_ptr or other custom smart pointers instead of raw pointers. And you probably should.

This avoids a potentially costly refactor that completely avoids casting. Casting is there to be used as a tool.

Cons:

It is conceptually simple. This does not provide a strong set of vocabulary (methods, classes and patterns) for building a smart set of tools for building advanced type mechanics.

Likely whether or not something is hostile should be a data member or implemented as series of methods controlling instance behavior.

Someone might think that the pointers this returns convey ownership and delete them.

Every caller must check pointers for validity prior to use. Or you can add methods to check, but then callers will need to call methods to check before the cast. Checks like these are surprising for users of the class and make it harder to use correctly.

It is polymorphism dense. This will perplex people who are uncomfortable with polymorphism. Even today there are many who are not comfortable with polymorphism.

A refactor that completely avoids casting is possible. Casting is dangerous and not a tool to use lightly.