Multiclassing and updating each of its component classes whenever it changes

Question

A Multiclass character can consist of any number of component classes. For example, if Fighter, Wizard, and Monk are character classes, then a Multiclass can be all 3 and having all three capabilities. Because multiple inheritance is impossible due to the unlimited number of such component classes, I'm using the Composite Pattern.

Now watch what happens here:

#include <iostream>
#include <cmath>
#include <vector>

struct Coordinates {
    int x, y, z;
    void print() const {std::cout << "(" << x << ", " << y << ", " << z << ").\n";}
};

struct LivingBeing {
    std::string name;
    int hitPoints;
    Coordinates coordinates = {0,0,0};
    LivingBeing (const std::string& n, int hp) : name(n), hitPoints(hp) {}
    virtual ~LivingBeing() = default;
    void moves (const Coordinates& position) {coordinates = position;  std::cout << name << " moves to ";  coordinates.print();}
    void showCoordinates() const {std::cout << name << "'s coordinates are ";  coordinates.print();}
};

struct CharacterClass : LivingBeing {
    using LivingBeing::LivingBeing;
};

struct Fighter : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Wizard : CharacterClass {
    using CharacterClass::CharacterClass;
    void castsFireBall (LivingBeing&);
};

struct Monk : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Multiclass : CharacterClass {
    std::vector<CharacterClass*> componentClasses;  // Composite Pattern for the component classes
    using CharacterClass::CharacterClass;
    Multiclass (const std::string& name, int hitPoints) : CharacterClass (name, hitPoints) {
        // For simpicity, assume Fighter/Wizard/Monk is the Multiclass.
        componentClasses.push_back (new Fighter(name, hitPoints));
        componentClasses.push_back (new Wizard(name, hitPoints));
        componentClasses.push_back (new Monk(name, hitPoints));
    }
    Wizard* getWizard() const {return static_cast<Wizard*>(componentClasses[1]);}
    void castsFireBall (LivingBeing& target) {getWizard()->castsFireBall(target);}
};

inline int distanceBetween (const Coordinates& c1, const Coordinates& c2) {
    return static_cast<int> ( sqrt (pow(c1.x - c2.x, 2) + pow(c1.y - c2.y, 2) + pow(c1.z - c2.z, 2)) );
}

inline int distanceBetween (const LivingBeing& being1, const LivingBeing& being2) {
    return distanceBetween (being1.coordinates, being2.coordinates);
}

void Wizard::castsFireBall (LivingBeing& target) {
    if (distanceBetween(*this, target) < 10) {
        std::cout << name << " is caught in his own fireball because his coordinates are \n";
        coordinates.print();
    }
    else
        std::cout << target.name << " is hit by the fireball.\n";
}

int main() {
    Multiclass rex ("Rex", 50);
    Fighter borg ("Borg", 80);

    borg.showCoordinates();
    rex.moves ({100, 120, 0});
    rex.showCoordinates();

    rex.castsFireBall(borg);
}

Output:

Borg's coordinates are (0, 0, 0).
Rex moves to (100, 120, 0).
Rex's coordinates are (100, 120, 0).
Rex is caught in his own fireball because his coordinates are
(0, 0, 0).

Rex, the Multiclass, moves far away enough from his target to cast his Wizard's fireball, yet he is still caught in the fireball. This error occurs because the coordinates used refer to the Wizard component of the Multiclass, not the Multiclass itself.

First solution: Add a Multiclass* data member to CharacterClass and use that when appropriate.

#include <iostream>
#include <cmath>
#include <vector>

struct Coordinates {
    int x, y, z;
    void print() const {std::cout << "(" << x << ", " << y << ", " << z << ").\n";}
};

struct LivingBeing {
    std::string name;
    int hitPoints;
    Coordinates coordinates = {0,0,0};
    LivingBeing (const std::string& n, int hp) : name(n), hitPoints(hp) {}
    virtual ~LivingBeing() = default;
    void moves (const Coordinates& position) {coordinates = position;  std::cout << name << " moves to ";  coordinates.print();}
    void showCoordinates() const {std::cout << name << "'s coordinates are ";  coordinates.print();}
};

struct CharacterClass : LivingBeing {
    struct Multiclass* multiclass = nullptr;  // *** Add this data member.
    using LivingBeing::LivingBeing;
};

struct Fighter : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Wizard : CharacterClass {
    using CharacterClass::CharacterClass;
    void castsFireBall (LivingBeing&);
};

struct Monk : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Multiclass : CharacterClass {
    std::vector<CharacterClass*> componentClasses;
    using CharacterClass::CharacterClass;
    Multiclass (const std::string& name, int hitPoints) : CharacterClass (name, hitPoints) {
        Fighter* fighter = new Fighter(name, hitPoints);
        Wizard* wizard = new Wizard(name, hitPoints);
        Monk* monk = new Monk(name, hitPoints);
        componentClasses.push_back(fighter);
        componentClasses.push_back(wizard);
        componentClasses.push_back(monk);
        fighter->multiclass = this;
        wizard->multiclass = this;
        monk->multiclass = this;
    }
    Wizard* getWizard() const {return static_cast<Wizard*>(componentClasses[1]);}
    void castsFireBall (LivingBeing& target) {getWizard()->castsFireBall(target);}
};

inline int distanceBetween (const Coordinates& c1, const Coordinates& c2) {
    return static_cast<int> ( sqrt (pow(c1.x - c2.x, 2) + pow(c1.y - c2.y, 2) + pow(c1.z - c2.z, 2)) );
}

inline int distanceBetween (const LivingBeing& being1, const LivingBeing& being2) {
    return distanceBetween (being1.coordinates, being2.coordinates);
}

void Wizard::castsFireBall (LivingBeing& target) {
    const LivingBeing* spellCaster = multiclass ? static_cast<LivingBeing*>(multiclass) : static_cast<LivingBeing*>(this);  // The key line.
    if (distanceBetween(*spellCaster, target) < 10) {
        std::cout << name << " is caught in his own fireball because his coordinates are \n";
        coordinates.print();
    }
    else
        std::cout << target.name << " is hit by the fireball.\n";
}

int main() {
    Multiclass rex ("Rex", 50);
    Fighter borg ("Borg", 80);

    borg.showCoordinates();
    rex.moves ({100, 120, 0});
    rex.showCoordinates();

    rex.castsFireBall(borg);
}

Output:

Borg's coordinates are (0, 0, 0).
Rex moves to (100, 120, 0).
Rex's coordinates are (100, 120, 0).
Borg is hit by the fireball.

Drawbacks: The line

LivingBeing* spellCaster = multiclass ? static_cast<LivingBeing*>(multiclass) : static_cast<LivingBeing*>(this);

is certainly ugly. And this needs to be done in all similar functions, and it is easy forget to make this check, leading to bugs that will be hard to find if the check is ever missed.

Second solution: Change the data of each component class immediately from LivingBeing::moves(const Coordinates&) itself:

#include <iostream>
#include <cmath>
#include <vector>

struct Coordinates {
    int x, y, z;
    void print() const {std::cout << "(" << x << ", " << y << ", " << z << ").\n";}
};

struct LivingBeing {
    std::string name;
    int hitPoints;
    Coordinates coordinates = {0,0,0};
    LivingBeing (const std::string& n, int hp) : name(n), hitPoints(hp) {}
    virtual ~LivingBeing() = default;
    void moves (const Coordinates&);
    void showCoordinates() const {std::cout << name << "'s coordinates are ";  coordinates.print();}
};

struct CharacterClass : LivingBeing {
    using LivingBeing::LivingBeing;
};

struct Fighter : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Wizard : CharacterClass {
    using CharacterClass::CharacterClass;
    void castsFireBall (LivingBeing&);
};

struct Monk : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Multiclass : CharacterClass {
    std::vector<CharacterClass*> componentClasses;
    using CharacterClass::CharacterClass;
    Multiclass (const std::string& name, int hitPoints) : CharacterClass (name, hitPoints) {
        componentClasses.push_back (new Fighter(name, hitPoints));
        componentClasses.push_back (new Wizard(name, hitPoints));
        componentClasses.push_back (new Monk(name, hitPoints));
    }
    Wizard* getWizard() const {return static_cast<Wizard*>(componentClasses[1]);}
    void castsFireBall (LivingBeing& target) {getWizard()->castsFireBall(target);}
};

inline int distanceBetween (const Coordinates& c1, const Coordinates& c2) {
    return static_cast<int> ( sqrt (pow(c1.x - c2.x, 2) + pow(c1.y - c2.y, 2) + pow(c1.z - c2.z, 2)) );
}

inline int distanceBetween (const LivingBeing& being1, const LivingBeing& being2) {
    return distanceBetween (being1.coordinates, being2.coordinates);
}

// *** Changed
void LivingBeing::moves (const Coordinates& position) {
    coordinates = position;
    std::cout << name << " moves to ";  coordinates.print();
    Multiclass* multiclass = dynamic_cast<Multiclass*>(this);
    if (multiclass) {
        for (CharacterClass* x : multiclass->componentClasses)
            x->moves(position);
    }
}

void Wizard::castsFireBall (LivingBeing& target) {
    if (distanceBetween(*this, target) < 10) {
        std::cout << name << " is caught in his own fireball because his coordinates are \n";
        coordinates.print();
    }
    else
        std::cout << target.name << " is hit by the fireball.\n";
}

int main() {
    Multiclass rex ("Rex", 50);
    Fighter borg ("Borg", 80);

    borg.showCoordinates();
    rex.moves ({100, 120, 0});
    rex.showCoordinates();

    rex.castsFireBall(borg);
}

Output:

Borg's coordinates are (0, 0, 0).
Rex moves to (100, 120, 0).
Rex moves to (100, 120, 0).  // 3 redundant lines from the component classes.
Rex moves to (100, 120, 0).
Rex moves to (100, 120, 0).
Rex's coordinates are (100, 120, 0).
Borg is hit by the fireball.

The main drawback is that the Multiclass* multiclass = dynamic_cast<Multiclass*>(this); check (followed by whatever changes to all of its component classes) needs to be done in every LivingBeing method that changes its data, which will certainly be plenty, and are also easy to forget to do. Furthermore, notice the 3 redundant lines in the output caused by the component classes.

At this point, I don't know which solution is better, and am displeased by both of them actually. Another solution may be to use the Observer Pattern, but I think it will be just like the second solution. I'm working on a solution using Publish and Subscribe right now, but feedback is certainly appreciated.

Third solution: Make LivingBeing::setCoordinates(const Coordinates&) virtual and overridden by Multiclass:

#include <iostream>
#include <cmath>
#include <vector>

struct Coordinates {
    int x, y, z;
    void print() const {std::cout << "(" << x << ", " << y << ", " << z << ").\n";}
};

struct LivingBeing {
    std::string name;
    int hitPoints;
    Coordinates coordinates = {0,0,0};
    LivingBeing (const std::string& n, int hp) : name(n), hitPoints(hp) {}
    virtual ~LivingBeing() = default;
    virtual void setCoordinates (const Coordinates& position) {coordinates = position;}
    void moves (const Coordinates& position) {
        setCoordinates(position);  // *** Must use this, even though 'coordinates = position;' also compiles!
        std::cout << name << " moves to ";  coordinates.print();
    }
    void showCoordinates() const {std::cout << name << "'s coordinates are ";  coordinates.print();}
};

struct CharacterClass : LivingBeing {
    using LivingBeing::LivingBeing;
};

struct Fighter : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Wizard : CharacterClass {
    using CharacterClass::CharacterClass;
    void castsFireBall (LivingBeing&);
};

struct Monk : CharacterClass {
    using CharacterClass::CharacterClass;
};

struct Multiclass : CharacterClass {
    std::vector<CharacterClass*> componentClasses;
    using CharacterClass::CharacterClass;
    Multiclass (const std::string& name, int hitPoints) : CharacterClass (name, hitPoints) {
        componentClasses.push_back (new Fighter(name, hitPoints));
        componentClasses.push_back (new Wizard(name, hitPoints));
        componentClasses.push_back (new Monk(name, hitPoints));
    }
    virtual void setCoordinates (const Coordinates&) override;
    Wizard* getWizard() const {return static_cast<Wizard*>(componentClasses[1]);}
    void castsFireBall (LivingBeing& target) {getWizard()->castsFireBall(target);}
};

inline int distanceBetween (const Coordinates& c1, const Coordinates& c2) {
    return static_cast<int> ( sqrt (pow(c1.x - c2.x, 2) + pow(c1.y - c2.y, 2) + pow(c1.z - c2.z, 2)) );
}

inline int distanceBetween (const LivingBeing& being1, const LivingBeing& being2) {
    return distanceBetween (being1.coordinates, being2.coordinates);
}

void Wizard::castsFireBall (LivingBeing& target) {
    if (distanceBetween(*this, target) < 10) {
        std::cout << name << " is caught in his own fireball because his coordinates are \n";
        coordinates.print();
    }
    else
        std::cout << target.name << " is hit by the fireball.\n";
}

void Multiclass::setCoordinates (const Coordinates& position) {  // *** NEW
    LivingBeing::setCoordinates(position);
    for (CharacterClass* x : componentClasses)
        x->setCoordinates(position);
}

int main() {
    Multiclass rex ("Rex", 50);
    Fighter borg ("Borg", 80);

    borg.showCoordinates();
    rex.moves ({100, 120, 0});
    rex.showCoordinates();

    rex.castsFireBall(borg);
}

Output:

Borg's coordinates are (0, 0, 0).
Rex moves to (100, 120, 0).
Rex's coordinates are (100, 120, 0).
Borg is hit by the fireball.

This solution is an improvement over the second solution because it avoids the dynamic_cast and thus performs a bit faster and avoids the three unwanted output lines because it only carries out the change in coordinates itself. One still must remember to write an override for every "set" method of LivingBeing though. Even more easy to forget is to use the "set" method within EVERY LivingBeing function, because a LivingBeing function does not require using the "set" method to compile, unless there is a way to force that somehow.

In trying to employ the 3rd solution concisely for all such functions in Multiclass, the following helper function won't compile though (due to some syntax problem):

template <typename T>
inline void Multiclass::setDataMember (const T& newValue, void(LivingBeing::*f)(const T&)) {
    (this->LivingBeing::*f)(newValue);  // GCC 4.8.1 won't accept this syntax. Template disambiguator needed?
    for (CharacterClass* x : componentClasses)
        (x->*f)(newValue);
}

Answer 1

Code Review

You need to start looking at class/struct differenes

This looks fine as a structure:

struct Coordinates {
    int x, y, z;
    void print() const {std::cout << "(" << x << ", " << y << ", " << z << ").\n";}
};

But this looks more like a class

struct LivingBeing {
    std::string name;
    int hitPoints;

You don't want other people reaching and changing the hit points of a character without using one of the offical mechanisme: like

void LivingBeing::inflictDammage(int damage)
{
    if (itenary.find("MagicRingOfStinky") {  // If you don't go through the inflictDamage
                                             // interface then people may forget
                                             // to check for the stinky ring!!
        damage = max(0, damage-2);
    }
    hitPoints -= damage;
}

Print routines:

These are fine. But also add the standard operator<<() to make it easy to print to any stream.

 friend std::ostream& operator<<(std::ostream& str, Coordinates const& c)
 {
    c.print(str); // Need to modify print to take a stream.
    return str;
 }

Learn about smart pointers

Modern C++ does not use many RAW pointers (if you see them you are looking at people that are writting C with the C++ compiler). Your pointers should be held inside smart pointers (or resource aware objects of some type) that know how/when to deallocate them:

Here:

std::vector<CharacterClass*> componentClasses;

A vector of pointers (OK I can live with that).

    componentClasses.push_back (new Fighter(name, hitPoints));
    componentClasses.push_back (new Wizard(name, hitPoints));
    componentClasses.push_back (new Monk(name, hitPoints));

Pushing things created with new into it. Not that cool but I am looking for the point where these objects are destroyed and I see no destructor.

Design

I think you have some fundamental design flaws:

struct CharacterClass : LivingBeing {
    using LivingBeing::LivingBeing;
};

I think they basically stem from here. A character class is not a living being. A class is an attribute of sentient being that has taken the time to train for those abilities. i.e. A character may have "no class" as a child. But he trains with the local militia and gains some skills as a fighter (does not mean he "is-a" a fighter but rather has some skills as a fighter). Subsequently the character rescues a monk and as payment the monk imparts some training to the character that gives him more skills.

So to me the "class" of a character imparts some set of skills that a character can utilize when taking an action.

 class Character: public LivingBeing
 {
     std::vector<SkillSet*>   skills;
     public:
        // As character gets more training/experience we can add new
        // skills. Like FighterLevel1 skills FighterLevel2 skills
        // etc.
        void undertakeTraining(SkillSet& newSkills)
        {
            skills.push_back(&newSkills);
        }
        // When we perform an action we ask each skill we have learned
        // how well that skill can do the action (both Monk and Fighter
        // can attack with bare hands but a monk has a higher skill level
        // at unarmed combat, but a fighter has a higher skill level at
        // armed combat).
        //
        // So find the skill set with the highest profeciency at a task
        // and use that.
        //
        // Note: The default skill at an action is `defaultInabilityToDoAction`
        //       This will return `skillLevel()` of 0 (or something obviously low)
        //       If you use this SkillAtAction to perform the task it is going
        //       to not be affective. So if you try and perform the action "FireBall"
        //       and you have no wizzard skill training then you are not going to
        //       find any ability to perform the action.
        void performAction(Action const& action)
        {
            static NoSkillAtAction  defaultInabilityToDoAction;

            std::reference_wrapper<SkillAtAction>  skillAtAction = defaultInabilityToDoAction;

            for(SkillSet* skill: skills) {
                SkillAtAction&  canDoAction = skills.canDoAction(action);
                if (canDoAction.skillLevel() > skillAtActionskillLevel()) {
                    skillAtAction = canDoAction;
            }

            // Now Found the skill set that does the action at the highest Level
            skillAtAction.performAction(*this, action);
        }

    };

Other Classes you may need.

    class Action
    {
      public:
         virtual ~Action() {}
    };
    class SkillAtAction;
    class SkillSet
    {
        public:
            SkillAtAction& canDoAction(Action const& action);
    };
    class SkillAtAction
    {
        public:
            virtual int skillLevel() = 0;
            virtual void performAction(LivingBeing& src, Action const& action) = 0;
    };

    class defaultInabilityToDoAction: public SkillAtAction
    {
        public:
            virtual int skillLevel() {return 0;}
            virtual void performAction(LivingBeing& src, Action const& action)
            {
                std::cout << "What are you thinking you have no idea how to do that\n";
            }
    };

Answer 2

I think your code has become a tad complicated and it seems to violate the DRY principle since you have 3 copies of coordinate that you have to choose from.

A better way to think of your characters are that in fact, they are "roles". With this in mind it doesn't seem right for all the roles to have a "moves" method. Each role represents a different set of capabilities and the logic to determine which role's capabilities apply in a given situation should be in the Multiclass.

The challenge lies in roles being able to define their own actions and having it accessible through the Multiclass. If you had reflection, or were using a more dynamically typed language, this would be easy to solve, but in C++, we don't have this freedom.

What you could do is have an actions map structure that is defined for each role that allows an action method to be invoked using a string (key to the map is a string, value is a function handle/delegate). This does mean you have to use a standard interface for all actions but as long as the action has access to the Multiclass's state (e.g. a reference), you should be fine. The Multiclass would then have to define some logic to search through each role for a given action.