2
\$\begingroup\$

In Dungeons and Dragons, when one puts on multiple magical items, all of its effects will take place. These effects are to be cumulative, so if Girdle of Strength adds 2 to one's strength, and a Helmet of Power multiplies one's strength by 1.5, then wearing both will have both changes (and the order does matter), and when one of them is removed, that change must be removed to. StrengthEnhancedState could hold a map of functions with the map adding and removing elements (that is when the map look-up will be needed) when the items are put on or removed, but the ordering of elements of the map does not correspond to the ordering of the items according to when they were put on, so the map will not be composing the functions in the correct order. The map could be given a custom ordering predicate but then the functions will have to be paired with some time value to establish the ordering. To avoid this mess, I felt that the Chain of Responsibility pattern would be an appropriate tool to use here, with each new item put on be put at the end of the chain, thereby the

virtual void handle() { if (next) next->handle(); }

method will always carry out the operations in the correct order. Here is my code illustrating this:

#include <iostream>

class LivingBeing;

class CreatureModifier {
    CreatureModifier* next = nullptr;
protected:
    LivingBeing* creature;  // Nesteruk states that pointer or shared_ptr can be used instead of reference.  In this case, we will use pointer because it can be nullptr if it is a magical item that is not possessed or worn by anyone.
public:
    CreatureModifier (LivingBeing* being = nullptr) : creature(being) { }
    virtual void handle() {
        if (next) next->handle();  // handle() is meant to be overriden.
    }
    void setModifiedBeing (LivingBeing* being) { creature = being; }
    void add (CreatureModifier& cm) {
        if (next) next->add(cm);
        else next = &cm;
    }
    void remove (CreatureModifier& cm) {
        if (!next)
            return;
        if (next == &cm)
            next = next->next;
        else
            next->remove(cm);
    }
};

class LivingBeingState {
protected:
    LivingBeing& stateOwner;
    LivingBeingState (LivingBeing& being) : stateOwner(being) { }
};

class StrengthEnhancedState : public LivingBeingState {
    CreatureModifier creatureModifier;
    const int originalStrength;
public:
    StrengthEnhancedState (LivingBeing&);  // LivingBeingState(being), creatureModifier(&being), originalStrength(being.getStrength()) { }
    void addStrengthChange (CreatureModifier& cm) { creatureModifier.add(cm); }
    void removeStrengthChange (CreatureModifier& cm) { creatureModifier.remove(cm); }
    void createNewStrength();  // { stateOwner.setStrength(originalStrength);   creatureModifier.handle(); }
};

class MagicalItem { };

class ModifyingMagicalItem : public MagicalItem, public CreatureModifier { };

class StrengthModifyingMagicalItem : public ModifyingMagicalItem  { };

class LivingBeing {
private:
    std::string name;
    int strength;
    StrengthEnhancedState* strengthEnhancedState = nullptr;  // To be just one state pointer in a std::tuple of state pointers called 'simultaneousStates'.
public:
    LivingBeing() = default;
    LivingBeing (const std::string& n, int s) : name(n), strength(s) { }
    virtual ~LivingBeing() { delete strengthEnhancedState; }
    virtual int getStrength() const { return strength; }
    virtual void setStrength (int num) { strength = num; }
    void wear (StrengthModifyingMagicalItem& s) {
        if (!strengthEnhancedState)
            strengthEnhancedState = new StrengthEnhancedState(*this);
        s.setModifiedBeing(this);
        strengthEnhancedState->addStrengthChange(s);
        strengthEnhancedState->createNewStrength();  // CreatureModifier::handle() called within this, the trademark call of the Chain of Responsibility Pattern.
    }
    void takeOff (StrengthModifyingMagicalItem& s) {
        strengthEnhancedState->removeStrengthChange(s);
        strengthEnhancedState->createNewStrength();
    }
    void display (std::ostream& os = std::cout) const { os << name << "'s strength is " << strength << ".\n"; }
};

class GirdleOfStrength : public StrengthModifyingMagicalItem {
    void handle() override { creature->setStrength(creature->getStrength() + 2);  CreatureModifier::handle(); }
};

class HelmetOfPower : public StrengthModifyingMagicalItem {
    void handle() override { creature->setStrength(creature->getStrength() * 1.5);  CreatureModifier::handle(); }
};

StrengthEnhancedState::StrengthEnhancedState (LivingBeing& being) : LivingBeingState(being), creatureModifier(&being), originalStrength(being.getStrength()) { }

void StrengthEnhancedState::createNewStrength() { stateOwner.setStrength(originalStrength);  creatureModifier.handle(); }

int main() {
    LivingBeing fighter("Rex", 15);
    fighter.display();
    std::cout << "Adding strength enchancements:\n";
    GirdleOfStrength girdleOfStrength1, girdleOfStrength2;
    HelmetOfPower helmetOfPower;
    fighter.wear(girdleOfStrength1);  fighter.display();
    fighter.wear(helmetOfPower);  fighter.display();
    fighter.wear(girdleOfStrength2);  fighter.display();
    fighter.takeOff(helmetOfPower);  fighter.display();
}

Output:

Rex's strength is 15.
Adding strength enchancements:
Rex's strength is 17.
Rex's strength is 25.
Rex's strength is 27.
Rex's strength is 19.
\$\endgroup\$
1
  • 1
    \$\begingroup\$ I’m baffled by the need for all this complexity. If all you care about is the order of objects, all you need is a sequence container. Pretty much any sequence container will do. You say you need a “map look-up” for “adding and removing”… but that’s nonsense, and the proof of that is the fact that you are actually doing add and remove by just iterating linearly (unnecessarily in the case of add). Add is just modifiers.push_back(modifier) and remove is just std::erase(modifiers, modifier). \$\endgroup\$
    – indi
    Jan 21 at 22:07

1 Answer 1

3
\$\begingroup\$

This is just a linked list

In the chain-of-responsibility pattern, the idea is that each item can decide whether or not to take responsibility for whatever action it is that you want to perform. It then either will do that action and stop, or it won't take responsibility and instead just call the next item in the chain.

In your case, you just want to have every item in the chain do its thing. All items have a responsibility. You don't need to use the chain-of-responsibility pattern then, you can just make it a linked list of modifiers:

class CreatureModifier {
public:
    virtual void handle() = 0; // must be overridden
};

class StrengthEnhancedState : public LivingBeingState {
    std::forward_list<CreatureModifier*> modifiers;
    …
};

void StrengthEnhancedState::createNewStrength() {
    stateOwner.setStrength(originalStrength);
    for (auto modifier: modifiers) {
        modifier->handle();
    }
}

Note that you don't even have to use a linked list here; you could also use the more efficient std::vector.

The advantage of this approach is that it removes the responsibility of one modifier having to call the next in the chain. That makes the code simpler and easier to maintain.

Missing virtual destructors

If you have a class with virtual functions, you should always add a virtual destructor. Otherwise, deleting an object via a pointer to its base class will fail to call the actual destructor of the derived class. Your code might seem to work, but it actually has undefined behavior.

Avoid manual memory allocation

Avoid calling new and delete directly, and instead use smart pointers or containers where possible. So:

class LivingBeing {
    …
    std::unique_ptr<StrengthEnhancedState> strengthEnhancedState;
    …
public:
    virtual ~LivingBeing() { /* No delete necessary anymore */ }
    …
    void wear(StrengthModifyingMagicalItem& s) {
        if (!strengthEnhancedState)
            strengthEnhancedState =
                std::make_unique<StrengthEnhancedState>(*this);
        …
    }
    …
};

Be careful with object ownership

Consider this code:

LivingBeing fighter("Rex", 15);
{
    HelmetOfPower helmetOfPower;
    fighter.wear(helmetOfPower);
}

After this code, fighter thinks it is wearing a helmetOfPower, and has a pointer to it somewhere. However, the helmetOfPower object has already been destroyed. You might avoid this by letting fighter take ownership of objects it is wearing.

\$\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.