Working on a solid Command Pattern with shared_ptr

Question

I am trying to implement a very clean Command Pattern in a library.

I have the following structure right now (a few parts are still being finished up):

1. users (client-code) have some Object, call it "Manager"
2. Manager holds a collection of shared_ptr<Foo>
3. Manager provides access to the collection by returning shared_ptr<Foo>
4. I have a Command abstract class and a hierarchy of commands for actions to perform on Foo
5. Client code should not call Command::execute(), only Manager should, Manager::execute(shared_ptr<Command>), so that it can handle undo/redo

I would like to follow the following rules:

1. users (client-code) have some Object, call it "Manager"
2. Manager holds a collection of shared_ptr<Foo>
3. Manager provides access to the collection by returning shared_ptr<const Foo>
4. I have a Command abstract class and a hierarchy of commands for actions to perform on Foo
5. Client code cannot (without workarounds) call Command::execute(), only Manager can, Manager::execute(shared_ptr<Command>), so that it can handle undo/redo and get non-const smart pointers
6. A Manager must be able to allow Command objects to access and modify shared_ptr<Foo> even though the user initializes Command objecst with shared_ptr<const Foo>

I am just trying to figure out the best way to handle giving out shared_ptr<const Foo> while allowing number 5 and 6 to work.

Is there any example/design pattern that does this which I could learn from? Is this a good idea compared to what I already have/am working on?

Answer 1

I think this passkey pattern should be the right thing for you:

class CommandExecuteKey{
private:
  friend class Manager;
  CommandExecuteKey(){}
};

class Command{
public:
  // usual stuff
  void execute(CommandExecuteKey);
};

class Manager{
public
  void execute(Command& cmd){
    // do whatever you need to do
    cmd.execute(CommandExecuteKey());
  }
};

Now, Command doesn't know anything about Manager, only about the key that is needed for the execute function. The user won't be able to call the execute method directly, as only Manager can create CommandExecuteKey objects, thanks to a private constructor and friendship.

int main(){
  Command cmd;
  Manager mgr;
  //cmd.execute(CommandExecuteKey()); // error: constructor not accessible
  mgr.execute(cmd); // works fine, Manager can create a key
}

---

Now, for your 6th point:
When you get the command in, search all your shared_ptr<Foo>s for the correct object (using the saved shared_ptr of the command as a search-key) and then pass that mutable one from your internal shared_ptrs back to the command.

Answer 2

I'm not following your question 100%, but here goes...

The only thing I can think of for #5 is to make Command::execute private/protected and make Manager a friend of Command. The downside of this approach is that you've now introduced a dependency from Command to Manager.

As for #6, if the user's shared_ptr<const Foo> objects were originated from Manager's shared_ptr<Foo> collection, then Manager should be able to safely const_pointer_cast shared_ptr<const Foo*> back into shared_ptr<Foo*>. If Manager attempts to const cast a shared_ptr<const Foo*>, where the pointee is an actual constant object, you'll get undefined behavior.

---

I've thought of another solution for #5:

Define an ExecutableCommand class, derived from Command. ExecutableCommand has an added method to invoke the command, to be used only by Manager. Clients can only access ExecutableCommand objects via pointers/references to Command. When a Manager wants to invoke a Command, it downcasts it to a ExecutableCommand to gain access to the invocation interface.

Working example (including const_pointer_cast for #6):

#include <iostream>
#include <string>
#include <vector>
#include <boost/shared_ptr.hpp>

using namespace std;
using namespace boost;

//------------------------------------------------------------------------------
struct Foo
{
    Foo(int x) : x(x) {}
    void print() {++x; cout << "x = " << x << "\n";} // non-const
    int x;
};

//------------------------------------------------------------------------------
struct Command
{
    // Interface accessible to users
    std::string name;

private:
    virtual void execute() = 0;
};

//------------------------------------------------------------------------------
struct ExecutableCommand : public Command
{
    // Only accessible to Manager
    virtual void execute() {} // You may want to make this pure virtual
};

//------------------------------------------------------------------------------
struct PrintCommand : public ExecutableCommand
{
    PrintCommand(shared_ptr<const Foo> foo)
        : foo_( const_pointer_cast<Foo>(foo) ) {}

    void execute() {foo_->print();}

private:
    shared_ptr<Foo> foo_;
};

//------------------------------------------------------------------------------
struct Manager
{
    void execute(Command& command)
    {
        ExecutableCommand& ecmd = dynamic_cast<ExecutableCommand&>(command);
        ecmd.execute();
    }

    void addFoo(shared_ptr<Foo> foo) {fooVec.push_back(foo);}

    shared_ptr<const Foo> getFoo(size_t index) {return fooVec.at(index);}

private:
    std::vector< shared_ptr<Foo> > fooVec;
};

//------------------------------------------------------------------------------
int main()
{
    Manager mgr;
    mgr.addFoo( shared_ptr<Foo>(new Foo(41)) );

    Command* print = new PrintCommand(mgr.getFoo(0));
    // print.execute() // Not allowed
    mgr.execute(*print);

    delete print;
}

Answer 3

Since it wouldn't make any sense to me otherwise, I'm going to assume that

• your library provides the Manager class (or at least a base class), and
• clients have to use that class to invoke a Command.

In that case, maybe something like this could work:

void Manager::execute(Command& cmd, shared_ptr<Foo const> const& target)
{
    shared_ptr<Foo> mutableTarget = this->LookupMutableFoo(mutableTarget); // throws if not found
    cmd.execute(mutableTarget); // client cannot invoke without mutable pointer
}

// or, if the "target" needs to be stored in the Command you could use something like this:
void Manager::execute(Command& cmd)
{
    shared_ptr<Foo> mutableTarget = this->LookupMutableFoo(cmd.GetTarget()); // throws if not found
    cmd.execute(mutableTarget); // client cannot invoke without mutable pointer
}

I'm not sure though if using const is the best solution here. Maybe you should wrap your Foo objects in e.g. ClientFoo objects. The manager only hands out pointers to ClientFoo. The manager can then (e.g. via friend) get the Foo from the ClientFoo, and use it to invoke the Command.