Requests and handling requests

Question

For this task, I don't think that the Observer Pattern can be used in its classic form. Those who have received the requests and those who choose to accept the request are not the same group. Those who choose to accept the request could be viewed as the observers, and they can stop accepting the request at any time and so be unregistered as observers, but they will still have the request received. This means that we have two layers of observers, with one being a subset of another. Furthermore, there are different requests types, with template enum RequestType, and having a container of observers will only pertain to one type of request, which won't do the job. Of course, once the requester's problem is solved, everybody who received the request will have the request automatically removed, but those who are in the middle of handling the request must be made to stop too. So this is how I handled it.

enum RequestType { FreeMeFromWeb, HelpMeGetUp };
struct HandleRequestBase;

class Request {
protected:
    LivingBeing* requester;
public:
    Request (LivingBeing* r) : requester(r) { }
    LivingBeing* getRequester() const { return requester; }
    LivingBeing*& getRequester() { return requester; }
    virtual std::shared_ptr<HandleRequestBase> makeHandleRequest (LivingBeing* helper) = 0;
};

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

struct HandleRequestBase : Action {
    Request* request;
    HandleRequestBase (LivingBeing* helper, Request* r) : Action(helper), request(r) {}
    virtual ~HandleRequestBase() = default;
};

template <RequestType>
struct HandleRequest : HandleRequestBase {
    using HandleRequestBase::HandleRequestBase;
    virtual void execute() override;
};

template <RequestType R>
struct RequestClass : Request {
    static const std::string tag;
    using Request::Request;
    virtual std::shared_ptr<HandleRequestBase> makeHandleRequest (LivingBeing* helper) override { return std::make_shared<HandleRequest<R>>(helper, this); }
};
template <> const std::string RequestClass<FreeMeFromWeb>::tag = "FreeMeFromWebRequest";
template <> const std::string RequestClass<HelpMeGetUp>::tag = "HelpMeGetUpRequest";

class LivingBeing {
    std::list<std::shared_ptr<Request>> requestsReceived;
    std::unordered_map<RequestType, std::shared_ptr<Request>> requestsMade;
    ...
    const std::list<std::shared_ptr<Request>>& getRequestsReceived() const { return requestsReceived; }
    void receiveRequest (const std::shared_ptr<Request>& request) { requestsReceived.push_back(request); }
    void removeRequestReceived (const std::shared_ptr<Request>& request) { requestsReceived.remove(request); }
private:
    virtual Menu buildMenu();
};

template <> void HandleRequest<FreeMeFromWeb>::execute() {
    std::cout << "\nACTION TAKEN:  " << actionTaker->getName() << " helps " << request->requester->getName() << " untangle him from the web.\n";
    std::cout << "It may take some time.\n";
}

inline void LivingBeing::removeCaughtInWebSpellState() {
    std::unordered_map<RequestType, std::shared_ptr<Request>>::const_iterator it = requestsMade.find(FreeMeFromWeb);
    std::shared_ptr<Request> request = it != requestsMade.end() ? it->second : nullptr;
    if (request)
        requestsMade.erase(FreeMeFromWeb);  // 'this' removes his request made to untangle from the web now that he no longer needs the help, whether the request was followed by anyone or not.
    std::cout << name << " is now free from the web.\n";
    if (!request)
        return;
    for (LivingBeing* being : allBeingsPresent) {  // Anyone who might have been helping 'requester' to untangle from the web shall celebrate.
        auto it = std::find(being->getRequestsReceived().begin(), being->getRequestsReceived().end(), request);
        if (it != being->getRequestsReceived().end())
            being->celebrate();
    }
    for (LivingBeing* being : allBeingsPresent)
        being->removeRequestReceived(request);  // Everyone present calls 'removeRequestReceived(request)' in case others received the same request (for those who didn't receive the request, then nothing will happen for them).  
}

// Within, StatesMediator::buildMenu(), there is:
if (!subject->getRequestsReceived().empty()) {  // 'subject' is the LivingBeing that the menu of actions is being built for.
    for (const std::shared_ptr<Request>& request : subject->getRequestsReceived()) {
        const std::shared_ptr<HandleRequestBase> handleRequest = request->makeHandleRequest(subject);
        const std::shared_ptr<Option> option = std::make_shared<Option>(handleRequest, handleRequest->description());
        menu.addOption(option); 
    }
}

// Then there is (AskForHelpToUntangleFromWeb derives from Action):
void AskForHelpToUntangleFromWeb::execute()  {
    std::cout << "\nACTION TAKEN:  " << actionTaker->getName() << " asks " << helper->getName() << " to help untangle him from the web.\n";
    auto it = actionTaker->requestsMade.find(FreeMeFromWeb);
    if (it == actionTaker->requestsMade.end()) {
        std::shared_ptr<RequestClass<FreeMeFromWeb>> request = std::make_shared<RequestClass<FreeMeFromWeb>>(actionTaker);
        actionTaker->requestsMade[FreeMeFromWeb] = request;
        helper->receiveRequest(request);
    }
    else
        helper->receiveRequest(it->second);
}

// And within Menu StatesMediator::buildMenu(), there is:
for (LivingBeing* being : allBeingsPresent) {
    if (caughtInWebSpellState->getHelpers().empty()) {
        for (LivingBeing* being : allBeingsPresent) {
            if (being == subject || being->isDead() || being->isIncapacitated())
                continue;
            auto it = subject->requestsMade.find(FreeMeFromWeb);
            if (it != subject->requestsMade.end()) {  // i.e. 'subject' has already requested for help to get out of the web.
                std::shared_ptr<Request> request = it->second;
                auto j = std::find(being->getRequestsReceived().begin(), being->getRequestsReceived().end(), request);
                if (j != being->getRequestsReceived().end())  // 'being' is already helping 'subject', so no need for 'subject' to ask 'being' again (even if 'being' stopped helping 'subject', 'being' still has the request from 'subject' in his menu of options).
                    continue;
            }
            std::shared_ptr<Option> option = std::make_shared<Option>(std::make_shared<AskForHelpToUntangleFromWeb>(subject, being), "Ask " + being->getName() + " for help to free him from the web.");
            menu.addOption(option);
        }
    }
}

Here I could only illlustrate the main ideas I used. I cannot give a fully compiling code as it would be too long fit in here and to read.

Answer 1

Using design patterns

For this task, I don't think that the Observer Pattern cannot be used in its classic form.

I'm assuming the double negative there was not intentional. Indeed, each software design patterns is a tool to solve a specific problem. If it doesn't match the problem exactly, you probably shouldn't use it as is, otherwise you are trying tighten screws using a hammer.

Those who choose to accept the request could be viewed as the observers, and they can stop accepting the request at any time and so be unregistered as observers, but they will still have the request received. This means that we have two layers of observers, with one being a subset of another.

But not a strict subset. It's more like a Venn diagram with two circles partially overlapping, the two circles being:

1. Observers that accept new requests
2. Observers that have outstanding accepted requests

If I understand correctly, multiple observers might accept the same kind of request, but once one completes the request, the other observers that accepted that same request should stop working on that request. This is the part that can can be solved in different ways.

One way would be to have some shared object associated with the request from the subject; each observer gets a pointer to that object. This shared object is used to record the state of the request; there should be an atomic way to query whether the request has already been fulfilled and to record that an observer has fulfilled the request.

Another way would be to split each request type into two; one for starting a request and one for stopping a request. Each observer would initially subscribe to both. Once an observer has accepted a request, it can unsubscribe from the "start request" messages, but it must still listen for "stop request" messages. When one observer finishes the request of the subject, the subject sends the "stop request" to listening observers, and the remaining ones can then cancel their work, and only then unsubscribe from the "stop request" messages. You might need to use unique IDs to match "stop request" messages to the corresponding "start request" messages.

Consider removing the non-const getRequester()

Are you sure you want someone to be able to change the requester pointer after constructing a Request? This seems like a dangerous thing to do, it might be better to remove the non-const overload of getRequester().

Use more auto

You can avoid spelling out typenames in many situations by using auto. For example, instead of:

std::unordered_map<RequestType, std::shared_ptr<Request>>::const_iterator it = requestsMade.find(FreeMeFromWeb);

Just write:

auto it = requestsMade.find(FreeMeFromWeb);

I also recommend you use auto in range-for loops. Apart from less typing and cleaner looking code, it can also prevent some types or errors (think accidentily using the wrong typename and an undesired implicit cast causing it to compile without a warning).

Use if with initializer if possible

Since C++17, if-statements can have an initializer in them. For example:

if (auto it = requestsMade.find(FreeMeFromWeb); it != requestsMade.end()) {
    ...
}

Inefficient copying of std::shared_ptrs

Making a copy of a std::shared_ptr is not free, since atomic reference counts need to be updated, so avoid doing that. There are two ways to avoid that, the first is to std::move() it, like so:

if (auto it = requestsMade.find(FreeMeFromWeb); it != requestsMade.end()) {
    auto request = std::move(it->second);
    requestsMade.erase(FreeMeFromWeb);
    ...
}

However, even simpler is to not make a copy of it, just use the element in the map, and only erase it after you are done with it:

if (auto it = requestsMade.find(FreeMeFromWeb); it != requestsMade.end()) {
    auto& request = it->second;
    ...
    requestsMade.erase(FreeMeFromWeb);
}

I can't see the declaration of Menu::addOption(), but if it takes a std::shared_ptr<Option> as an argument by value, then the following it highly inefficient:

const std::shared_ptr<Option> option = std::make_shared<Option>(handleRequest, handleRequest->description());
menu.addOption(option); 

As probably that means a copy is made when in that last line, and I am guessing another one will be made inside addOption(). Making the variable option non-const allows you to std::move() it into addOption(). Or write this instead:

menu.addOption(std::make_shared<Option>(handleRequest, handleRequest->description());

Answer 2

Here is a fully compiling code using the two-layered Observer Pattern suggested by G. Sliepen. There are two separate lists of observers, those who received the requests and those who accepted the requests. Instead of the receivers storing the requests received, the requests received will store the receivers.

#include <iostream>
#include <list>
#include <vector>
#include <unordered_map>
#include <memory>

struct LivingBeing;

enum RequestType { FreeMeFromWeb, HelpMeGetUp, LendMeSomeMoney, NumRequestTypes };
struct HandleRequestBase;

class Request {
protected:
    LivingBeing* requester;
public:
    Request (LivingBeing* r) : requester(r) { }
    LivingBeing* getRequester() const { return requester; }
    virtual std::shared_ptr<HandleRequestBase> makeHandleRequest (LivingBeing* helper) = 0;
    virtual void registerReceivedRequestObserver (LivingBeing*) = 0;
    virtual void unregisterReceivedRequestObserver (LivingBeing*) = 0;
    virtual void registerAcceptedRequestObserver (LivingBeing*) = 0;
    virtual void unregisterAcceptedRequestObserver (LivingBeing*) = 0;
    virtual void notifyReceivedRequestObservers() = 0;
    virtual void notifyAcceptedRequestObservers() = 0;
    virtual void newRoundUpdate() = 0;
    virtual void terminate() = 0;
};

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

struct HandleRequestBase : Action {
    Request* request;
    HandleRequestBase (LivingBeing* helper, Request* r) : Action(helper), request(r) {}
    virtual ~HandleRequestBase() = default;
};

template <RequestType>
struct HandleRequest : HandleRequestBase {
    using HandleRequestBase::HandleRequestBase;
    virtual void execute() override;
};

template <RequestType R>
struct RequestClass : Request {
    std::list<LivingBeing*> receivedRequestObservers, acceptedRequestObservers;  // *** Two categories of observers needed.
    static const std::string tag;
    using Request::Request;
    virtual ~RequestClass() { terminate(); }
    virtual std::shared_ptr<HandleRequestBase> makeHandleRequest (LivingBeing* helper) override { return std::make_shared<HandleRequest<R>>(helper, this); }
    virtual void registerReceivedRequestObserver (LivingBeing* being) override { receivedRequestObservers.push_back(being); }
    virtual void unregisterReceivedRequestObserver (LivingBeing* being) override { receivedRequestObservers.remove(being); }
    virtual void registerAcceptedRequestObserver (LivingBeing* being) override { acceptedRequestObservers.push_back(being); }
    virtual void unregisterAcceptedRequestObserver (LivingBeing* being) override { acceptedRequestObservers.remove(being); }
private:
    virtual void notifyReceivedRequestObservers() override;
    virtual void notifyAcceptedRequestObservers() override;
    virtual void terminate() override;
    virtual void newRoundUpdate() override;
};

struct AskForHelpToUntangleFromWeb : Action {
    LivingBeing* helper;
    AskForHelpToUntangleFromWeb (LivingBeing* tangledBeing, LivingBeing* h) : Action(tangledBeing), helper(h) { }
    virtual void execute() override;
};

struct LivingBeing {
    std::string name;
    std::vector<std::shared_ptr<Action>> menuOptions;
//  std::list<std::shared_ptr<Request>> requestsReceived;  // ****** We don't need this anymore if using the Observer Pattern.
//  ****** Instead of the LivingBeing receivers storing the requests received, the requests received will store the LivingBeing receivers.
    std::unordered_map<RequestType, std::shared_ptr<Request>> requestsMade;
    LivingBeing (const std::string& name) : name(name) { }
    std::string getName() const { return name; }
    void receiveRequest (const std::shared_ptr<Request>& request) {
        request->registerReceivedRequestObserver(this);  // *** Register as a receivedRequestObserver.
    }
    void addMenuOption (const std::shared_ptr<Action>& action, const std::string& description) {
        menuOptions.push_back(action);
        std::cout << "Menu option added for " << name << ":  " << description << '\n';
    }
    template <typename T> void removeMenuOption (LivingBeing*) { menuOptions.pop_back(); }  // Keeping it simple for now.  Need to search for all menu options with type T associated.
    void celebrates() { std::cout << name << " celebrates.  Job accomplished!\n";  }
};

template <RequestType R>
void RequestClass<R>::notifyReceivedRequestObservers() {
    for (LivingBeing* observer : receivedRequestObservers)
        observer->addMenuOption(std::make_shared<HandleRequest<R>>(observer, this), "Help " + requester->getName() + " untangle from the web.");
}

template <RequestType R>
void RequestClass<R>::notifyAcceptedRequestObservers() {
    for (LivingBeing* observer : acceptedRequestObservers)
        std::cout << observer->getName() << " is notified that " << requester->getName() << " is still entangled in the web.\n";  // Or whatever requester's current state is.
}

template <RequestType R>
void RequestClass<R>::terminate() {
    for (LivingBeing* observer : receivedRequestObservers) {
        observer->removeMenuOption<HandleRequest<R>>(requester);  // Everyone who received the request will have the menu option of accepting the request removed.
        std::cout << requester->getName() << " no longer needs " << observer->getName() << "'s help.\n";
    }
    for (LivingBeing* observer : acceptedRequestObservers) {
        observer->celebrates();
        std::cout << requester->getName() << " thanks " << observer->getName() << " for helping.\n";
    }
}

template <RequestType R>
void RequestClass<R>::newRoundUpdate() {
    notifyAcceptedRequestObservers();
    acceptedRequestObservers.clear();  // Instead of removing an acceptedRequestObserver when he chooses to quit (which is not so straightforward to implement),
    // simply remove all acceptedRequestObservers and register them back only if they choose to continue the job in the next round.
}

template <>
void HandleRequest<FreeMeFromWeb>::execute() {
    static int numAttempts = 0;
    std::cout << "ACTION TAKEN:  " << actionTaker->getName() << " helps " << request->getRequester()->getName() << " untangle him from the web.\n\n";
    request->registerAcceptedRequestObserver(actionTaker);  // *** Register as an acceptedRequestObserver.
    numAttempts++;
    if (numAttempts >= 4) {  // Let's assume the attempt succeeds after 4 tries.
        std::cout << actionTaker->getName() << " has succeeded!  " << request->getRequester()->getName() << " is now free from the web.\n";
        request->getRequester()->requestsMade.erase(FreeMeFromWeb);  // RequestClass<FreeMeFromWeb>::terminate() called through its destructor.
    }
}

void AskForHelpToUntangleFromWeb::execute()  {
    std::cout << "ACTION TAKEN:  " << actionTaker->getName() << " asks " << helper->getName() << " to help untangle him from the web.\n\n";
    auto it = actionTaker->requestsMade.find(FreeMeFromWeb);
    if (it == actionTaker->requestsMade.end()) {
        std::shared_ptr<RequestClass<FreeMeFromWeb>> request = std::make_shared<RequestClass<FreeMeFromWeb>>(actionTaker);
        actionTaker->requestsMade[FreeMeFromWeb] = request;
        helper->receiveRequest(request);
    }
    else
        helper->receiveRequest(it->second);
}

std::list<LivingBeing*> allBeingsPresent;

void newRound() {
    static int roundNumber = 0;
    std::cout << "Round " << ++roundNumber << '\n';
    for (LivingBeing* being : allBeingsPresent) {
        for (auto& pair : being->requestsMade)
            pair.second->newRoundUpdate();  // Every request (of any type) from every member of allBeingsPresent calls newRoundUpdate().
    }
}

int main() {
    LivingBeing *goblin = new LivingBeing("Ugly Goblin"), *borg = new LivingBeing("Borg"), *rex = new LivingBeing("Rex"), *merlin = new LivingBeing("Merlin");
    allBeingsPresent = {goblin, borg, rex, merlin};
    
    AskForHelpToUntangleFromWeb askBorgForHelp(goblin, borg);
    AskForHelpToUntangleFromWeb askRexForHelp(goblin, rex);
    AskForHelpToUntangleFromWeb askMerlinForHelp(goblin, merlin);

    newRound();
    askBorgForHelp.execute();  askRexForHelp.execute();   askMerlinForHelp.execute();  // Assume these menu options were chosen by 'goblin'.
    goblin->requestsMade.at(FreeMeFromWeb)->notifyReceivedRequestObservers();  // The request itself must be stored somewhere, in this case, in the 'goblin->requestsMade' map (with FreeMeFromWeb as key).
    std::cout << "borg->menuOptions.size() = " << borg->menuOptions.size() << '\n';  // Menu option of helping 'goblin' added.
    std::cout << "rex->menuOptions.size() = " << rex->menuOptions.size() << '\n';  // Menu option of helping 'goblin' added.
    std::cout << "merlin->menuOptions.size() = " << merlin->menuOptions.size() << '\n';  // Menu option of helping 'goblin' added.
    borg->menuOptions.back()->execute();  // Assume 'borg' chooses this menu option, to help 'goblin'.
    merlin->menuOptions.back()->execute();  // Assume 'merlin' chooses this menu option, to help 'goblin'.

    newRound();
    borg->menuOptions.back()->execute();  // 'borg' chooses to continue helping 'goblin' (but 'merlin' does not, though he still has the option to).
    rex->menuOptions.back()->execute();  // 'rex' chooses to start helping 'goblin'.
    // rex succeeds, but only borg and rex celebrate, since merlin stopped helping.

    std::cout << "borg->menuOptions.size() = " << borg->menuOptions.size() << '\n';  // Menu option of helping 'goblin' removed.
    std::cout << "rex->menuOptions.size() = " << rex->menuOptions.size() << '\n';  // Menu option of helping 'goblin' removed.
    std::cout << "merlin->menuOptions.size() = " << merlin->menuOptions.size() << '\n';  // Menu option of helping 'goblin' removed.
    std::cin.get();
}

Output:

Round 1
ACTION TAKEN:  Ugly Goblin asks Borg to help untangle him from the web.

ACTION TAKEN:  Ugly Goblin asks Rex to help untangle him from the web.

ACTION TAKEN:  Ugly Goblin asks Merlin to help untangle him from the web.

Menu option added for Borg:  Help Ugly Goblin untangle from the web.
Menu option added for Rex:  Help Ugly Goblin untangle from the web.
Menu option added for Merlin:  Help Ugly Goblin untangle from the web.
borg->menuOptions.size() = 1
rex->menuOptions.size() = 1
merlin->menuOptions.size() = 1
ACTION TAKEN:  Borg helps Ugly Goblin untangle him from the web.

ACTION TAKEN:  Merlin helps Ugly Goblin untangle him from the web.

Round 2
Borg is notified that Ugly Goblin is still entangled in the web.
Merlin is notified that Ugly Goblin is still entangled in the web.
ACTION TAKEN:  Borg helps Ugly Goblin untangle him from the web.

ACTION TAKEN:  Rex helps Ugly Goblin untangle him from the web.

Rex has succeeded!  Ugly Goblin is now free from the web.
Ugly Goblin no longer needs Borg's help.
Ugly Goblin no longer needs Rex's help.
Ugly Goblin no longer needs Merlin's help.
Borg celebrates.  Job accomplished!
Ugly Goblin thanks Borg for helping.
Rex celebrates.  Job accomplished!
Ugly Goblin thanks Rex for helping.
borg->menuOptions.size() = 0
rex->menuOptions.size() = 0
merlin->menuOptions.size() = 0