I am a bit worried about the mutability of your command parameters. Is it really necessary to create a command with constantly changing parameters?

Problems with your approach:

Do you want other threads/commands to change your parameters while perform is going on?

Do you want visitBefore and visitAfter of the same Command object to be called with different DIParameter objects?

Do you want someone to feed parameters to your commands, that the commands have no idea about?

None of this is prohibited by your current design. While a generic key-value parameter concept has its merits at times, I do not like it with respect to a generic command class.

Example of consequences:

Consider a concrete realization of your Command class – something like CreateUserCommand. Now obviously, when you request a new user to be created, the command will need a name for that user. Given that I know the CreateUserCommand and the DIParameters classes, which parameter should I set?

I could set the userName parameter, or username.. do you treat parameters case insensitively? I wouldn’t know really.. oh wait.. maybe it’s just name?

As you can see the freedom you gain from a generic key-value mapping implies that using your classes as someone who did not implement them is unwarrantedly difficult. You would at least need to provide some constants for your commands to let others know which keys are supported by that command.

Possible different designs approaches:

• Immutable parameters: By turning your Parameter instances immutable you can freely reuse them among different commands.
• Specific parameter classes: Given a UserParameter class that contains exactly the parameters I would need for commands that involve a user, it would be much simpler to work with this API. You could still have inheritance on the parameters, but it would not make sense any longer for command classes to take arbitrary parameters – at the pro side of course this means that API users know which parameters are required exactly.
• One command instance per context: If you need your commands to have things like visitBefore and visitAfter, whilst also reusing them with different parameters, you will be open to the problem of getting called with differing parameters. If the parameters should be the same on multiple method calls, you need to encapsulate them into the command such that they cannot be switched out for other parameters in-between the calls.

What is nice about design principles is that sooner or later, they conflict with each other.

In the situation described, I think I would prefer to go with some kind of context that each command can take information from and put information to (especially if those are key-value pairs). This is based on a trade off : I do not want separate commands to be coupled just because they are some kind of input to each other. Inside CommandB, I do not care how UserName was set – just that it is there for me to use. Same thing in CommandA : I set the information in, I do not want to know what other are doing with it – neither who they are.

This means a sort of passing context, which you can find bad. For me, the alternative is worse, especially if this simple key-value context (can be a simple bean with getters and setters, to limit a bit the “free form” factor) can allow the solution to be simple and testable, with well separated commands, each with its own business logic.

Lets assume you have a command interface:

class Command {
public:
    void execute() = 0;
};

And a subject:

class Subject {
    std::string name;
public:
    void setName(const std::string& name) {this->name = name;}
}

What you need is:

class NameObserver {
public:
    void update(const std::string& name) = 0;
};

class Subject {
    NameObserver& o;
    std::string name;
private:
    void setName(const std::string& name) {
        this->name = name;
        o.update(name);
    }
};

class CommandB: public Command, public NameObserver {
    std::string name;
public:
    void execute();
    void update(const std::string& name) {
        this->name = name;
        execute();
    }
};

Set NameObserver& o as a reference to CommandB. Now whenever CommandA changes the Subjects name CommandB can execute with the correct information. If name is used by more command use a std::list<NameObserver>

I don’t know if this is the right way to handle this on Programmers (in which case I apologize), but, after having checked all the answers here (@Frank’s in particular). I refactored my code this way:

• Dropped DIParameters. I’ll have individual (generic) objects as DIOperation’s input (immutable). Example:

class RelatedObjectTriplet {
private:
    std::string const m_sPrimaryObjectId;
    std::string const m_sSecondaryObjectId;
    std::string const m_sRelationObjectId;

    RelatedObjectTriplet& operator=(RelatedObjectTriplet other);

public:
    RelatedObjectTriplet(std::string const& sPrimaryObjectId,
                         std::string const& sSecondaryObjectId,
                         std::string const& sRelationObjectId);

    RelatedObjectTriplet(RelatedObjectTriplet const& other);


    std::string const& getPrimaryObjectId() const;
    std::string const& getSecondaryObjectId() const;
    std::string const& getRelationObjectId() const;

    ~RelatedObjectTriplet();
};

• New DIOperation class (with example) defined as:

template <class T = void> 
class DIOperation {
public:
    virtual int perform() = 0;

    virtual T getResult() = 0;

    virtual ~DIOperation() = 0;
};

class CreateRelation : public DIOperation<RelatedObjectTriplet> {
private:
    static std::string const TYPE;

    // Params (immutable)
    RelatedObjectTriplet const m_sParams;

    // Hidden
    CreateRelation & operator=(CreateRelation const& source);
    CreateRelation (CreateRelation const& source);

    // Internal
    std::string m_sNewRelationId;

public:
    CreateRelation(RelatedObjectTriplet const& params);

    int perform();

    RelatedObjectTriplet getResult();

    ~CreateRelation();
};

• It can be used like this:

RelatedObjectTriplet triplet("33333", "55555", "77777");
CreateRelation createRel(triplet);
createRel.perform();
const RelatedObjectTriplet res = createRel.getResult();

Thanks for the help and I hope I haven’t made mistakes here 🙂