2
\$\begingroup\$

A Multiclass is a CharacterClass that decided to add new CharacterClass types to combine with the original CharacterClass. These new CharacterClass types are stored in a std::vector<CharacterClass*> data member of CharacterClass called otherClasses. Common operations are used through iterating through originalClass, and hence the Composite Pattern is being used. Furthermore, each of these multiclass component classes have a CharacterClass* data member called originalClass, which points to the original CharacterClass. This pointer originalClass is needed so that these muticlass components will forward to originalClass whenever data of CharacterClass is being used or changed, and thus the Proxy Pattern is being used, as they are in a sense a proxy for originalClass in terms of usage of these CharacterClass data, but note that they do not act as proxies when using data and methods unique to themselves and not accessible from CharacterClass.

#include <iostream>
#include <vector>
#include <map>
#include <functional>
#include <set>
#include <memory>
#include <algorithm>

enum CharacterClassType { FIGHTER, WIZARD, CLERIC, NUM_CHARACTER_TYPES };

static bool ask_yn (const std::string& question) {
    while (true) {
        std::cout << question << " (y/n) ";
        char yesNo;
        if (std::cin >> yesNo) {
            switch (yesNo) {
                case 'y': case 'Y': return true;
                case 'n': case 'N': return false;
            }
        }
        else {  // Input failed. Assume no.
            std::cout << '\n';
            return false;
        }
    }
}

class LivingBeing {
    std::string name;
    int hitPoints;
    std::vector<struct Weapon*> weaponsCarried;
public:
    LivingBeing (const std::string& n) : name(n), hitPoints(10) { }
    LivingBeing() = default;
    virtual ~LivingBeing() = default;
    virtual std::string getName() const { return name; }
    virtual int getHitPoints() const { return hitPoints; }
    virtual void changeHitPointsBy (int change) { hitPoints += change; }
    virtual void displayChoices() = 0;
    virtual const std::vector<Weapon*>& getWeaponsCarried() const { return weaponsCarried; }
    virtual void addWeapon (Weapon* weapon) { weaponsCarried.push_back(weapon); }
    virtual bool canUseWeapon (const Weapon*) const = 0;
    virtual int checkForSpecialAttackBonus (Weapon*, const LivingBeing&) const = 0;
};

// Action classes
class Action {
protected:
    LivingBeing* actionTaker;
public:
    Action (LivingBeing* being) : actionTaker(being) { }
    virtual ~Action() = default;
    virtual void execute() = 0;
};

struct Option {
    std::shared_ptr<Action> action;
    std::string description;
    Option (const std::shared_ptr<Action>& a, const std::string& d) : action(a), description(d) { }
    bool operator< (const Option& other) const { return description < other.description; }
};

// Two Options will be considered equal if their description's are equal.
struct OptionPointerCompare {
    bool operator()(const Option* a, const Option* b) const { return *a < *b; }
    bool operator()(const std::shared_ptr<Option>& a, const std::shared_ptr<Option>& b) const { return operator()(a.get(), b.get()); }
};

class Attack : public Action {
public:
    using Action::Action;
    virtual void execute() override;
};

template <typename T>
class CastSpell : public Action {
public:
    using Action::Action;
    virtual void execute() override;  // { dynamic_cast<T*>(actionTaker)->castASpell(); }
};

class TurnAwayUndead : public Action {
public:
    using Action::Action;
    virtual void execute() override;  // { dynamic_cast<Cleric*>(actionTaker)->turnsAwayUndead(); }
};

class DoNothing : public Action {
public:
    using Action::Action;
    virtual void execute() override { std::cout << actionTaker->getName() << " does nothing.\n"; }
};

template <CharacterClassType> class Character;
using Fighter = Character<FIGHTER>;
using Wizard = Character<WIZARD>;
using Cleric = Character<CLERIC>;

class CharacterClass : public LivingBeing {
    int level = 1;
    CharacterClass* originalClass = nullptr;  // Proxy Pattern.  If *this is a new class added to a multiclass whose original class is originalClass, then *this is a proxy for originalClass in terms of hit points adjustment, weapons used, equipment carried, etc...
    std::vector<CharacterClass*> otherClasses;  // To handle multiclassing, using the Composite Pattern.
    static const std::map<CharacterClassType, std::function<CharacterClass*(CharacterClass*)>> multiclassFactory;
public:
    using LivingBeing::LivingBeing;
    CharacterClass (CharacterClass* b) : originalClass(b) { }  // Used only when multiclassing with a new CharacterClass type.
    void levelUp();
    virtual CharacterClassType getCharacterClassType() const = 0;
    virtual std::string getName() const override { return originalClass ? originalClass->getName() : LivingBeing::getName(); }  // PropertyProxy<T> can handle this automatically.
    virtual int getHitPoints() const override { return originalClass ? originalClass->getHitPoints() : LivingBeing::getHitPoints(); }
    virtual void changeHitPointsBy (int change) override { return originalClass ? originalClass->changeHitPointsBy(change) : LivingBeing::changeHitPointsBy(change); }
    virtual const std::vector<Weapon*>& getWeaponsCarried() const override { return originalClass ? originalClass->getWeaponsCarried() : LivingBeing::getWeaponsCarried(); }
    virtual void addWeapon (Weapon* weapon) override { return originalClass ? originalClass->addWeapon(weapon) : LivingBeing::addWeapon(weapon); }
    bool isMultiClass() const { return !otherClasses.empty(); }
    bool isMultiClassComponent() const { return originalClass != nullptr; }  // i.e. *this is just a component class of originalClass.
    int getMultiClassLevel() const { return level; }
    int getLevel() const {
        return !isMultiClass() ? level : level + std::accumulate(otherClasses.begin(), otherClasses.end(), 0,
            [](int x, const CharacterClass* c) {return x + c->getMultiClassLevel();});  // level + Sum of all levels through otherClasses (Composite Pattern).
    }
    virtual void displayChoices() override;
    // Testing checking whether a weapon can be used if a member of otherClasses can use it (even if *this cannot use it).
    virtual bool canUseWeapon (const Weapon* weapon) const override {
        const std::vector<CharacterClass*>& myClasses = isMultiClassComponent() ? originalClass->otherClasses : otherClasses;
        // Composite Pattern, using canUse(const Weapon*) through otherClasses after using canUse on itself.
        return canUse(weapon) || std::any_of(myClasses.begin(), myClasses.end(), [weapon](const CharacterClass* c)->bool { return c->canUse(weapon); });
    }
    virtual bool canUse (const Weapon* weapon) const = 0;
    virtual std::string className() const = 0;
    // Testing checking for bonus attack values among otherClasses (even if *this has no bonus attack value).
    virtual int checkForSpecialAttackBonus (Weapon* weapon, const LivingBeing& target) const override {
        int bonus = specialAttackBonus(weapon, target);
        for (const CharacterClass* c : otherClasses)  // Composite Pattern, using specialAttackBonus through otherClasses after using specialAttackBonus on itself.
            bonus = std::max(bonus, c->specialAttackBonus(weapon, target));
        return bonus;
    }
protected:
    virtual void gainNewLevelAttributes() = 0;
private:
    virtual void getSpecificChoices (std::set<std::shared_ptr<Option>, OptionPointerCompare>&) = 0;
    void increaseLevel() { level++;  gainNewLevelAttributes(); }
    void displayMulticlassLevels (std::ostream& = std::cout) const;
    std::string characterClassName (CharacterClassType c) const { return std::unique_ptr<CharacterClass>(multiclassFactory.at(c)(nullptr))->className(); }  // nullptr passed to prevent initializeAtLevelOne() being called.
    virtual int specialAttackBonus (const Weapon*, const LivingBeing&) const = 0;
};

struct Weapon {
    virtual std::string getName() const = 0;
    bool canBeUsedBy (const CharacterClass*) const { return true; }
    virtual bool canBeUsedBy (const Wizard*) const { return true; }  // Overridden by Sword, which Wizards cannot use.
    virtual bool canBeUsedBy (const Cleric*) const { return true; }  // Overridden by SharpWeapon, which Clerics cannot use.
};

struct SharpWeapon : Weapon {
    virtual bool canBeUsedBy (const Cleric*) const { return false; }
};

struct Sword : SharpWeapon {
    std::string getName() const override { return "sword"; }
    virtual bool canBeUsedBy (const Wizard*) const { return false; }
};

struct Dagger : SharpWeapon { std::string getName() const override { return "dagger"; } };

struct Mace : Weapon { std::string getName() const override { return "mace"; } };

template <>
class Character<FIGHTER> : public CharacterClass {
public:
    Character (const std::string& name) : CharacterClass(name) { initializeAtLevelOne(); }
    Character (CharacterClass* originalClass) : CharacterClass(originalClass) { if (originalClass) initializeAtLevelOne(); }
private:
    virtual std::string className() const override { return "Fighter"; }
    virtual CharacterClassType getCharacterClassType() const override { return FIGHTER; }
    virtual void getSpecificChoices (std::set<std::shared_ptr<Option>, OptionPointerCompare>& choices) override {
        choices.emplace(std::make_shared<Option>(std::make_shared<Attack>(this), "Attack"));
        choices.emplace(std::make_shared<Option>(std::make_shared<DoNothing>(this), "Do nothing"));
    }
    void initializeAtLevelOne() { std::cout << getName() << " has been initialized as a level 1 fighter.\n"; }
    virtual void gainNewLevelAttributes() override { std::cout << "New fighter attributes (e.g. weapon proficiencies) given to " << getName() << " after levelling up.\n"; }
    bool canUse (const Weapon* weapon) const { return weapon->canBeUsedBy(this); }  // double dispatch
    virtual int specialAttackBonus (const Weapon* weapon, const LivingBeing&) const override {
        if (getMultiClassLevel() >= 2 && dynamic_cast<const Sword*>(weapon) != nullptr) {
            std::cout << "Special attack bonus of 3 given for Fighters at level 2+ with swords.\n";
            return 3;
        }
        return 0;
    }
};

template <>
class Character<WIZARD> : public CharacterClass {
    int numSpells = 0;
public:
    Character (const std::string& name) : CharacterClass(name) { initializeAtLevelOne(); }
    Character (CharacterClass* originalClass) : CharacterClass(originalClass) { if (originalClass) initializeAtLevelOne(); }
    void castASpell() {
        std::cout << numSpells << " Wizard spells available, and one of them is cast.\n";
        numSpells--;
        std::cout << getName() << " has " << numSpells << " Wizard spells left.\n";
        // The following CharacterClass:changeHitPointsBy(int) call is to demonstrate that originalClass's hit points (as opposed to the Wizard's hit points)
        // is changed even if the Wizard is a multiclass but not the original class (Proxy Pattern).
        std::cout << getName() << "'s fireball has caused damage to himself!  8 hit points lost!\n";
        changeHitPointsBy(-8);  // originalClass's hit points change (instead of the Wizard's hit points) if originalClass != nullptr.
        std::cout << getName() << " has " << getHitPoints() << " hit points left.\n";
    }
private:
    virtual std::string className() const override { return "Wizard"; }
    virtual CharacterClassType getCharacterClassType() const override { return WIZARD; }
    virtual void getSpecificChoices (std::set<std::shared_ptr<Option>, OptionPointerCompare>& choices) override {
        choices.emplace(std::make_shared<Option>(std::make_shared<Attack>(this), "Attack"));
        choices.emplace(std::make_shared<Option>(std::make_shared<CastSpell<Wizard>>(this), "Cast a Wizard spell"));
        choices.emplace(std::make_shared<Option>(std::make_shared<DoNothing>(this), "Do nothing"));
    }
    void initializeAtLevelOne() { numSpells = 3;  std::cout << getName() << " has " << numSpells << " wizard spells.\n"; }
    virtual void gainNewLevelAttributes() override { numSpells += 3;  std::cout << getName() << " now has " << numSpells << " wizard spells.\n"; }
    bool canUse (const Weapon* weapon) const { return weapon->canBeUsedBy(this); }
    virtual int specialAttackBonus (const Weapon*, const LivingBeing&) const override { return 0; }
};

template <>
class Character<CLERIC> : public CharacterClass {
    int numSpells = 0;
public:
    Character (const std::string& name) : CharacterClass(name) { initializeAtLevelOne(); }
    Character (CharacterClass* originalClass) : CharacterClass(originalClass) { if (originalClass) initializeAtLevelOne(); }
    void castASpell() {
        std::cout << numSpells << " Cleric spells available, and one of them is cast.\n";
        numSpells--;
        std::cout << getName() << " has " << numSpells << " Cleric spells left.\n";
    }
    void turnsAwayUndead() { std::cout << getName() << " turns away all undead present.\n"; }
private:
    virtual std::string className() const override { return "Cleric"; }
    virtual CharacterClassType getCharacterClassType() const override { return CLERIC; }
    virtual void getSpecificChoices (std::set<std::shared_ptr<Option>, OptionPointerCompare>& choices) override {
        choices.emplace(std::make_shared<Option>(std::make_shared<Attack>(this), "Attack"));
        choices.emplace(std::make_shared<Option>(std::make_shared<CastSpell<Cleric>>(this), "Cast a Cleric spell"));
        choices.emplace(std::make_shared<Option>(std::make_shared<TurnAwayUndead>(this), "Turn away undead"));
        choices.emplace(std::make_shared<Option>(std::make_shared<DoNothing>(this), "Do nothing"));
    }
    void initializeAtLevelOne() { numSpells = 2;  std::cout << getName() << " has " << numSpells << " cleric spells.\n"; }
    virtual void gainNewLevelAttributes() override { numSpells += 2;  std::cout << getName() << " now has " << numSpells << " cleric spells.\n"; }
    bool canUse (const Weapon* weapon) const { return weapon->canBeUsedBy(this); }
    virtual int specialAttackBonus (const Weapon*, const LivingBeing&) const override { return 0; }
};

const std::map<CharacterClassType, std::function<CharacterClass*(CharacterClass*)>> CharacterClass::multiclassFactory = {
    { FIGHTER, [](CharacterClass* originalClass) { return new Fighter(originalClass); } },
    { WIZARD, [](CharacterClass* originalClass) { return new Wizard(originalClass); } },
    { CLERIC, [](CharacterClass* originalClass) { return new Cleric(originalClass); } }
};

void CharacterClass::levelUp() {
    changeHitPointsBy(5);
    std::cout << '\n' << getName() << " has leveled up, and is now has " << getHitPoints() << " hit points.\n";
    if (otherClasses.size() == NUM_CHARACTER_TYPES - 1 || ask_yn("Does " + getName() + " wish to multiclass with a new class as he levels up?") == false) {
        // No new class is being added, but if *this is already a multiclass, we must ask which class he wants to level up in.
        if (otherClasses.size() > 0) {
            std::vector<CharacterClass*> choices;
            int choiceNumber = 2;
            std::cout << "Which class does " << getName() << " wish to level up in?\n";
            std::cout << "1. " << className() << '\n';  // The original class
            for (CharacterClass* c : otherClasses) {
                std::cout << choiceNumber++ << ". " << c->className() << '\n';
                choices.push_back(c);
            }
            std::cin >> choiceNumber;
            if (choiceNumber == 1)
                increaseLevel();
            else
                choices[choiceNumber-2]->increaseLevel();;
            std::cout << getName() << "'s new levels are:\n";
            displayMulticlassLevels();
        }
        else {
            increaseLevel();
            std::cout << getName() << "' is now at level " << level << ".\n";
        }
        return;
    }
    std::cout << "What new class does " << getName() << " with to multiclass with?\n";
    std::vector<CharacterClassType> characterClassChoices;
    int choiceNumber = 1;
    for (int i = 0;  i < NUM_CHARACTER_TYPES;  ++i) {
        const CharacterClassType c = static_cast<CharacterClassType>(i);
        if (c == getCharacterClassType())
            continue;
        if (std::none_of(otherClasses.begin(), otherClasses.end(), [c](const CharacterClass* cc)->bool { return cc->getCharacterClassType() == c; })) {
            characterClassChoices.push_back(c);
            std::cout << choiceNumber++ << ". " << characterClassName(c) << '\n';
        }
    }
    std::cin >> choiceNumber;
    const CharacterClassType type = characterClassChoices[choiceNumber-1];
    CharacterClass* newClass = multiclassFactory.at(type)(this);  // 'this' is passed and set as the 'originalClass' data member of newClass.
    otherClasses.push_back(newClass);
    std::cout << getName() << "'s new levels are:\n";
    displayMulticlassLevels();
}

void CharacterClass::displayChoices() {
      // Some Options will be identical among 'otherClasses', e.g. Attack or DoNothing option, so we must avoid the repeitions using std::set<std::shared_ptr<Option>, OptionPointerCompare>.
    std::set<std::shared_ptr<Option>, OptionPointerCompare> choices;
    std::cout << "\nWhat does " << getName() << " wish to do?\n";
    getSpecificChoices(choices);
    for (CharacterClass* c : otherClasses)
        c->getSpecificChoices(choices);
    std::vector<std::shared_ptr<Option>> menu(choices.begin(), choices.end());
    int choiceNumber = 1;
    for (const auto& c : choices)
        std::cout << choiceNumber++ << ". " << c->description << '\n';
    std::cin >> choiceNumber;
    menu[choiceNumber-1]->action->execute();
}

void CharacterClass::displayMulticlassLevels (std::ostream& os) const {
    os << className() << ": level " << level << '\n';  // Original class
    for (const CharacterClass* c : otherClasses)
        os << c->className() << ": level " << c->level << '\n';
    std::cout << "Total level: " << getLevel() << '\n';
}

void Attack::execute() {
    Fighter enemy("Enemy");  // Suppose 'enemy' is the target.
    std::cout << "Which weapon does " << actionTaker->getName() << " wish to use to attack " << enemy.getName() << "?\n";
    for (const Weapon* weapon : actionTaker->getWeaponsCarried()) {
        if (actionTaker->canUseWeapon(weapon))  // If is a multiclass, actionTaker's 'otherClasses' are checked if 'weapon' can be used, even if 'actionTaker' himself cannot use it.
            std::cout << weapon->getName() << '\n';
    }
    Weapon* weaponUsed = actionTaker->getWeaponsCarried().back();  // Going simple for now.
    const int attackBonus = actionTaker->checkForSpecialAttackBonus(weaponUsed, enemy);  // If is a multiclass, actionTaker's 'otherClasses' are checked as well as 'actionTaker' himself.
    if (attackBonus != 0)
        std::cout << "Attack bonus of " << attackBonus << " used.\n";
    // And similarly for other special attributes from actionTaker's 'otherClasses', if actionTaker is a multiclass.
}

template <typename T>
void CastSpell<T>::execute() { dynamic_cast<T*>(actionTaker)->castASpell(); }

void TurnAwayUndead::execute() { dynamic_cast<Cleric*>(actionTaker)->turnsAwayUndead(); }

void test (CharacterClass& c) {
    c.addWeapon(new Dagger);
    c.addWeapon(new Mace);
    c.addWeapon(new Sword);
    c.levelUp();
    c.levelUp();
    c.displayChoices();
    c.levelUp();
    c.displayChoices();
    c.displayChoices();
}

int main() {
//  Fighter fighter("Rex");
//  test(fighter);

    Wizard wizard("Merlin");
    test(wizard);

//  Cleric cleric("Luther");
//  test(cleric);
}

Sample output:

Merlin has 3 wizard spells.

Merlin has leveled up, and is now has 15 hit points.
Does Merlin wish to multiclass with a new class as he levels up? (y/n) y
What new class does Merlin with to multiclass with?
1. Fighter
2. Cleric
1
Merlin has been initialized as a level 1 fighter.
Merlin's new levels are:
Wizard: level 1
Fighter: level 1
Total level: 2

Merlin has leveled up, and is now has 20 hit points.
Does Merlin wish to multiclass with a new class as he levels up? (y/n) y
What new class does Merlin with to multiclass with?
1. Cleric
1
Merlin has 2 cleric spells.
Merlin's new levels are:
Wizard: level 1
Fighter: level 1
Cleric: level 1
Total level: 3

What does Merlin wish to do?
1. Attack
2. Cast a Cleric spell
3. Cast a Wizard spell
4. Do nothing
5. Turn away undead
2
2 Cleric spells available, and one of them is cast.
Merlin has 1 Cleric spells left.

Merlin has leveled up, and is now has 25 hit points.
Which class does Merlin wish to level up in?
1. Wizard
2. Fighter
3. Cleric
1
Merlin now has 6 wizard spells.
Merlin's new levels are:
Wizard: level 2
Fighter: level 1
Cleric: level 1
Total level: 4

What does Merlin wish to do?
1. Attack
2. Cast a Cleric spell
3. Cast a Wizard spell
4. Do nothing
5. Turn away undead
1
Enemy has been initialized as a level 1 fighter.
Which weapon does Merlin wish to use to attack Enemy?
dagger
mace
sword

What does Merlin wish to do?
1. Attack
2. Cast a Cleric spell
3. Cast a Wizard spell
4. Do nothing
5. Turn away undead
3
6 Wizard spells available, and one of them is cast.
Merlin has 5 Wizard spells left.
Merlin's fireball has caused damage to himself!  8 hit points lost!
Merlin has 17 hit points left.

Note that the overrides in CharacterClass::

virtual std::string getName() const override { return originalClass ? originalClass->getName() : LivingBeing::getName(); }
virtual int getHitPoints() const override { return originalClass ? originalClass->getHitPoints() : LivingBeing::getHitPoints(); }
virtual void changeHitPointsBy (int change) override { return originalClass ? originalClass->changeHitPointsBy(change) : LivingBeing::changeHitPointsBy(change); }

are a nuisance, though necessary. They can be handled automatically and the need for them being virtual removed by using a PropertyProxy class for those data members:

template <typename T>
struct PropertyProxy {
    using F = T(LivingBeing::*)() const;  // T get method
    using G = void(LivingBeing::*)(const T&);  // const T& set method
    T value;
    LivingBeing* being;
    F getPtrFunc;
    G setPtrFunc;
    PropertyProxy (const T initial_value) { *this = initial_value; }
    PropertyProxy (LivingBeing* b, F f) : being(b), getPtrFunc(f) { }
    PropertyProxy (LivingBeing* b, F f, G g) : being(b), getPtrFunc(f), setPtrFunc(g) { }
    PropertyProxy (const T initial_value, LivingBeing* b, F f) : value(initial_value), being(b), getPtrFunc(f) { }
    PropertyProxy (const T initial_value, LivingBeing* b, F f, G g) : value(initial_value), being(b), getPtrFunc(f), setPtrFunc(g) { }
    PropertyProxy() = default;
    operator T() const;
    T operator= (const T&);
    void operator+= (const T& t) { *this = *this + t; }
    void operator-= (const T& t) { *this = *this - t; }
    friend T operator+ (const T& t, const PropertyProxy<T>& p) { return t + p.value; }
    friend T operator- (const T& t, const PropertyProxy<T>& p) { return t - p.value; }
};

with

template <typename T>
PropertyProxy<T>::operator T() const {
    const CharacterClass* characterClass = dynamic_cast<const CharacterClass*>(being);
    if (!characterClass)
        return value;
    return characterClass->isMultiClassComponent() ? (characterClass->getOriginalClass()->*getPtrFunc)() : value;
}

template <typename T>
T PropertyProxy<T>::operator= (const T& new_value) {
    const CharacterClass* characterClass = dynamic_cast<const CharacterClass*>(being);
    if (!characterClass)
        return value = new_value;
    if (characterClass->isMultiClassComponent()) {
        (characterClass->getOriginalClass()->*setPtrFunc)(new_value);
        return new_value;   
    }
    return value = new_value;
}

Then changing the name and hitPoints data member of LivingBeing to

PropertyProxy<std::string> name{this, &LivingBeing::getName, &LivingBeing::setName};
PropertyProxy<int> hitPoints{this, &LivingBeing::getHitPoints, &LivingBeing::setHitPoints};

will do the job. However, this property proxy only works for primitive types, and not for types like std::vector and such that have their own methods. Hence, without having the full benefit desired, I decided to not use it at all and leave the original unchanged.

I should also mention that above is dynamic multiclassing, as opposed to static multiclassing: using Multiclass<OriginalClass, Ts...>, inheriting from OriginalClass, Ts..., is an example of the Static Decorator Pattern, and extreme case in fact, in the sense that each of OriginalClass, Ts... are decorated with data and functionality of all the other classes. Whenever a CharacterClass (which might already be a multiclass) decides to multiclass with a new type of CharacterClass, it is being decorated with the data and methods of that new type of CharacterClass. But using the static decorator pattern, as opposed to the dynamic decorator pattern, has its drawbacks, as a CharacterClass that is multiclassing will have to be turned in to a new type, and all pointers to the old type (which should be destroyed) will become invalid. Furthermore, the coding for it required so much template metaprogramming (and even multiple dispatching) that I didn't bother to show it for comparison. It is just a plain bad choice, and full of so many bugs that I gave up on trying to perfect it. The dynamic version I firmly believe is much better, and definitely much simpler.

\$\endgroup\$
5
  • \$\begingroup\$ Where the composite patern is used here? \$\endgroup\$
    – convert
    Commented Dec 25, 2022 at 15:03
  • \$\begingroup\$ @ convert CharacterClas has a std::vector<CharacterClass*> member, with each element iterated through for some methods I illustrated, e.g. canUse(Weapon*) and specialAttackBonus(). If the original character cannot use a sword, but one of the multiclass components can, then the overall character can use a sword. My sample output showed that (Merlin initially could not use a sword until multiclassing with a Fighter). \$\endgroup\$
    – prestokeys
    Commented Dec 25, 2022 at 15:29
  • \$\begingroup\$ But that´s not a composite patern like defined by Gof. \$\endgroup\$
    – convert
    Commented Dec 25, 2022 at 16:57
  • \$\begingroup\$ It matches that found on the internet, for example refactoring.guru/design-patterns/composite/cpp/example Perhaps Gof is out of date? I know that writer Nesteruk hates the Gof's state pattern. \$\endgroup\$
    – prestokeys
    Commented Dec 25, 2022 at 17:01
  • \$\begingroup\$ But when CharacterClas is your Component, where is your Composite extending it? \$\endgroup\$
    – convert
    Commented Dec 25, 2022 at 17:03

1 Answer 1

1
\$\begingroup\$

About the Overall Design

You probably should separate the logic or simulation and the presentation. A text based interface is fine; but putting cout and cin directly into your game logic makes for bad coupling.

Some of the design aspects feel a bit to over engineered OO. Like for example the action class. This class feels like it is not pulling it's weight. Maybe a string or enum as index into a map holding a callback function or just a switch statement would have done the job.

Likewise the Weapon struct should probably just be one struct that then contains multiple values instead of being a class hierarchy.

As a rule of thumb, try to implement the simplest thing you can get away with. Then if that does not work, extend the bit that needs extending. After all YANGI.

About the Code

For a start, you have a VERY dense formating. Now this is a matter of taste, but I would strongly suggest a formating with more white spaces to guide the eyes for better readability. I have decided after a long consideration to reformat the code and use the reformated code in my references.


enum CharacterClassType 
{ 
  FIGHTER, 
  WIZARD, 
  CLERIC, 
  NUM_CHARACTER_TYPES 
};

You should use enum class for any enum in modern C++. This prevents polluting the namespace. Also the NUM_CHARACTER_TYPES hack from C is not a good idea. You should not encode metadata in your data. If you must, put an additional constexpr next to it. But I would refrain from that, you generally don't iterate over an enum. If you iterate over something, it would be an array or vector of class data and then you should use the structure's size function.


static bool ask_yn (const std::string& question)

The static here has no sense. This is C++ not C and thus has no effect on visibility.


class LivingBeing 
{
public:
    LivingBeing() = default;

    LivingBeing (const std::string& n) 
    : name(n), hitPoints(10) {}

    virtual ~LivingBeing() = default;

private:
    std::string name;
    int hitPoints;
    std::vector<struct Weapon*> weaponsCarried;
};

You should initialize the hitPoints directly in the class. The problem is that the default constructor does not set a value resulting in undefined behavior. The name maybe can also be defaulted with something sensible. In you case maybe "Unnamed" makese sense. This will make more sense for logging; like Unnamed has leveled up. instead of has leveled up.

The word struct in struct Weapon has no sense. In C++ you do not need to repeat the struct keyword. Here you probably should use one of the standard smart pointers. I think unique_ptr might be the right one here. Right now you just leak the memory.

You should think about the copy constructor and assingment operator. I think copying may be sensible here. But as implemented you will get into trouble when you want to delete any weapon as they will have multiple strong references.

Not wrong, but LivingBeing sounds wired to my ears. It is common to call something like this the Actor in game engines. This allows for machines and mechanisms to also be actors.


class LivingBeing 
{
public:

    virtual std::string getName() const 
    { 
        return name; 
    }

    virtual int getHitPoints() const 
    {
        return hitPoints; 
    }

    virtual void changeHitPointsBy(int change) 
    { 
        hitPoints += change; 
    }

    virtual void displayChoices() = 0;

    virtual const std::vector<Weapon*>& getWeaponsCarried() const 
    { 
        return weaponsCarried; 
    }

    virtual void addWeapon(Weapon* weapon) 
    {
        weaponsCarried.push_back(weapon); 
    }

    virtual bool canUseWeapon(const Weapon*) const = 0;

    virtual int checkForSpecialAttackBonus(Weapon*, const LivingBeing&) const = 0;
};

Why are getName, getHiyPoints, changeHitPointsBy, getWeaponsCarried and addWeapon virtual? In what meaningful way can they be overridden?

In the case of getName you probably can return a const std::string&. Although not necessarily required, most get functions should have a symmetric set function. For example changeHitPointsBy is a sensible function, but it sigificantly limits semantics. It may be sensible to have a setHitPoints and then implement changeHitPointsBy in terms of that.

The same can be said with addWeapon. Any add function probably also needs a remove function.

The getWeaponsCarried function is fake const. Through the Weapon*, you can change the inner logic of the LivingBeing. A truly const function would have the signature std::vector<const Weapon*> getWeaponsCarried() const. And yes that requires creating a new vector and copying the pointers in. You could implement a int getWeaponCount() const and const Weapon* getWeapon(int) const API, but I don't like it. You probably should write the code in ways that makes getting weapons is the exception.

canUseWeapon probably should take a const Weapon&. Unless you need to pass in a pointer to transfer ownership or nullptr is an expected input, you should signal to the calling code this must be a valid reference by making it a reference.

Not entirely sure if checkForSpecialAttackBonus should be part of the class. The smells like it should be part of the Weapon class.


class Action 
{
public:
    Action(LivingBeing* being) 
    : actionTaker(being) {}

    virtual ~Action() = default;

    virtual void execute() = 0;

protected:
    LivingBeing* actionTaker;
};

The same comments of LivingBeing about initialization and copy semantics applies.

I am not sure if the actionTaker member is required here. Are there actions that may be not the source of an individual. It may also make sense for some actions to have a target additionally the source.


struct Option 
{
    std::shared_ptr<Action> action;
    std::string description;

    Option (const std::shared_ptr<Action>& a, const std::string& d) 
    : action(a), description(d) { }

    bool operator < (const Option& other) const 
    {
        return description < other.description; 
    }
};

To me the Option struct feels like it should be a class.

If you feel like a struct is the right thing, the constructor is actually overblown. You can remove it an still initialize the struct with uniform initialization.

In the same vain. The less operator should also probably be an operator defined outside of the struct.


struct OptionPointerCompare 
{
    bool operator () (const Option* a, const Option* b) const 
    {
      return *a < *b; 
    }

    bool operator () (const std::shared_ptr<Option>& a, const std::shared_ptr<Option>& b) const 
    {
        return operator()(a.get(), b.get()); 
    }
};

I am not sure if this a good idea. Pointers already have a functional less operator based on the address. This comparison means you are comparing strings for every comparison. I don't really see the sense in the other code that says the chices must be alphabetical.

The comparator for the shared_ptr should probably just be implemented as *a < *b. There is little benefit in first getting the raw pointer then passing it to the raw comparator, to then just do operator less.

If you need strict ordering, why not just use a std::vector of options instead?


class Attack : public Action 
{
public:
    using Action::Action;

    virtual void execute() override;
};

template <typename T>
class CastSpell : public Action 
{
public:
    using Action::Action;

    virtual void execute() override; 
};

class TurnAwayUndead : public Action {
public:
    using Action::Action;

    virtual void execute() override;
};

class DoNothing : public Action {
public:
    using Action::Action;

    virtual void execute() override 
    { 
      std::cout << actionTaker->getName() << " does nothing.\n"; 
    }
};

As a general rule, you define a virtual function with virtual and override it with override. It's either virtual or override, not both.

Using the parent's constructor is a nice way to reduce code. But I don't like it, it introduces a few unexpected side effects.

You are also inconsistent with you inlining of the code. This is probably because you are jamming everything into one cpp file. Maybe you should put the code into headers and cpp files.

template <typename T>
void CastSpell<T>::execute() 
{ 
    dynamic_cast<T*>(actionTaker)->castASpell(); 
}

This dynamic cast is entirely unnecessary. This is why I meant, that actionTaker probably should not be on the Action class.

Consider the following:

class Action 
{
public:
    virtual ~Action() = default;
    virtual void execute() = 0;
};

template <typename SpellCaster>
class CastSpell : public Action 
{
public:
    CastSpell(SpellCaster& s)
    : source(s) {}

    void execute() override
    {
        source.castASpell();
    } 

private:
    SpellCaster& source;
};

This makes sure that T must be of a type that can cast a spell and there is no uncertainty if actionTaker is of type T.

void TurnAwayUndead::execute() 
{ 
    dynamic_cast<Cleric*>(actionTaker)->turnsAwayUndead(); 
}

In this very same vain:

class TurnAwayUndead : public Action 
{
public:
    TurnAwayUndead(Cleric& c)
    : cleric(c) {}

    void execute() override
    {
        cleric.turnsAwayUndead();
    } 

private:
    Cleric& cleric;
};

void Attack::execute() 
{
    Fighter enemy("Enemy"); // Suppose 'enemy' is the target.
    
    std::cout << "Which weapon does " << actionTaker->getName() << " wish to use to attack " << enemy.getName() << "?\n";
    for (const Weapon* weapon : actionTaker->getWeaponsCarried()) {

        if (actionTaker->canUseWeapon(weapon))  
            // If is a multiclass, actionTaker's 'otherClasses' are checked if 'weapon' can be used, even if 'actionTaker' himself cannot use it.
            std::cout << weapon->getName() << '\n';
    }

    Weapon* weaponUsed = actionTaker->getWeaponsCarried().back();  // Going simple for now.
    const int attackBonus = actionTaker->checkForSpecialAttackBonus(weaponUsed, enemy);  // If is a multiclass, actionTaker's 'otherClasses' are checked as well as 'actionTaker' himself.
    if (attackBonus != 0)
        std::cout << "Attack bonus of " << attackBonus << " used.\n";
        // And similarly for other special attributes from actionTaker's 'otherClasses', if actionTaker is a multiclass.
}

So this function does allot and mixes consider splitting up the logic into bits that allows readers to understand the code without reading comments.

For example:

Weapon& Attack::selectWeapon(LivingBeing& target)
{
    std::cout << "Which weapon does " << actionTaker->getName() << " wish to use to attack " << target.getName() << "?\n";

    for (const Weapon* weapon : actionTaker->getWeaponsCarried()) 
    {
        if (actionTaker->canUseWeapon(weapon))  
        {
            std::cout << weapon->getName() << '\n';
        }
    }

    unsigned int choice = 0;
    std::cin >> choice; 
    // TODO somehow handle choice => wrapons.size()
    return actionTaker->getWeapon(choice);    
}

void Attack::applyDamage(Weapon& weapon, LivingBeing& target)
{
    const int attackBonus = actionTaker->checkForSpecialAttackBonus(weapon, target);
    if (attackBonus != 0)
    {
        std::cout << "Attack bonus of " << attackBonus << " used.\n";
    }

    auto damage = attackBonus + weapon.getDamage();
    target.changeHitPointsBy(-damage);
    weapon.applyWear(damage);
}

void Attack::execute() 
{
    Fighter enemy("Enemy");
    
    Weapon& weaponUsed = selectWeapon(enemy);
    applyDamage(weaponUsed, enemy);
}

struct Weapon 
{
    virtual std::string getName() const = 0;

    bool canBeUsedBy (const CharacterClass*) const 
    { 
        return true; 
    }
    virtual bool canBeUsedBy (const Wizard*) const 
    { 
        return true; 
    }

    virtual bool canBeUsedBy (const Cleric*) const 
    { 
        return true; 
    }
};

struct SharpWeapon : Weapon 
{
    virtual bool canBeUsedBy (const Cleric*) const 
    {
        return false;
    }
};

struct Sword : SharpWeapon 
{
    std::string getName() const override { 
        return "sword";
    }

    virtual bool canBeUsedBy (const Wizard*) const 
    { 
        return false; 
    }
};

struct Dagger : SharpWeapon 
{ 
    std::string getName() const override 
    {
        return "dagger"; 
    } 
};

struct Mace : Weapon 
{ 
    std::string getName() const override 
    {
        return "mace"; 
    } 
};

As stated in the design notes this is one of the places that feel like they are OO over engineered.

Consider the following:

#define BIT(n) (1 << (n))

enum class CharacterClass
{   
    FIGHTER = BIT(1), 
    WIZARD  = BIT(2), 
    CLERIC  = BIT(3)
};

constexpr CharacterClass operator | (CharacterClass rhs, CharacterClass lhs)
{
    return static_cast<CharacterClass>(static_cast<int>(rhs) | static_cast<int>(lhs));
}
constexpr CharacterClass operator & (CharacterClass rhs, CharacterClass lhs)
{
    return static_cast<CharacterClass>(static_cast<int>(rhs) & static_cast<int>(lhs));
}

struct Weapon 
{
    std::string     name;
    unsigned int    damage;
    CharacterClass  usableBy = FIGHTER | WIZARD | CLERIC;
};

constexpr Weapon sword  = {"sword",  15, FIGHTER};
constexpr Weapon dagger = {"dagger",  7, FIGHTER | WIZARD};
constexpr Weapon mace   = {"mace",   12};

Of course, you need to add special behavior to each weapon, you need a hierarchy of sorts. But you can still only add weapon types into the hierarchy and still map the specific weapons as data. (That you may have read from a file.)

In your code, for example the getName certainly does not need to be a virtual function and can be a data member on the base class.


class LivingBeing {};

class CharacterClass : public LivingBeing {};

template <CharacterClassType> class Character;

template <>
class Character<FIGHTER> : public CharacterClass {};

template <>
class Character<WIZARD> : public CharacterClass {};

template <>
class Character<CLERIC> : public CharacterClass {};

This use of template makes little sense. I see that this allows you to create Character<TPYE>, but to the compiler calling Character<FIGHTER> and Fighter makes not difference once everything is said and done. This approach just makes the compiler work longer and harder.

You already have the multiclassFactory as a means to a generic access, whats the benefit?


Speaking of factory. You probably should implemente a standalione factory function that encapsulates actor creation. Currently it's muddled into the CharacterClass

Consider the following:

std::shared_ptr<CharacterClass> createCharacter(CharacterClassType type, const std::stirng& name)
{
    switch (type)
    {
        case FIGHTER:
            return std::make_shared<Fighter>(name);
        case WIZARD:
            return std::make_shared<Wizard>(name);
        case CLERIC:
            return std::make_shared<Cleric>(name);
        default:
            assert(false);
            return nullptr;
    };
}

I don't quite like the implementation of getSpecificChoices. If you change the signature, you could do the following

std::vector<Option> Wizzard::getSpecificChoices()  
{
    return {
        {"Attack",              std::make_shared<Attack>(this)},
        {"Cast a Wizard spell", std::make_shared<CastSpell<Wizard>>(this)},
        {"Do nothing",          std::make_shared<DoNothing>(this)}
    };
}

There are few other minor things I would nitpick about, but these are the big ones.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.