Command handler executing commands with different dependencies

Question

Well basically, I have an Engine class that recieves a command as string from the input and passes it to a CommandHandler class which executes the apropriate command.

The CommandHandler passes the string to a CommandFactory to get the command and calls the method Execute() of the command, but the problem is that every command depends on different classes to execute properly. For example, one commands need the IOutputWriter to write something, the other needs IBuldingFactory to create a building, etc. I am using a reflection in the CommandFactory class, and I can't pass the dependencies through the constructor using Activator.CreateInstance(), because every command has different dependencies.

My current architecture looks something like this:

class Engine()
{
    IData data; // application database
    IInputReader inputReader;
    ICommandHandler commandHandler;

    public void Run()
    {
        string command = inputReader.Read();

        commandHandler.Handle(command, data);
    }
}

class CommandHandler()
{
    ICommandFactory commandFactory();

    public void Handle(string command, IData data)
    {
        string executableCommand = commandFactory().createCommand(command);
        executableCommand.Execute(data);
    }
}

class DisplayDataCommand : ICommand
{
    IOutputWriter outputWriter;        

    public void Execute(IData data)
    {
        outputWriter.Print(data.ToString());
    }
}

class BuildCommand : ICommand
{
    IBuildingFactory buildingFactory;
    string buildingType;

    public void Execute(IData data)
    {
        var building = buildingFactory.createBuilding(buildingType);

        data.AddBuilding(building);
    }
}

I can have different methods in the command handler for each command and call the appropriate method using switch case, but that would violate the Open/Closed principle. So my real question is - How to implement this without violating the Open/Closed principle.

Answer 1

The problem you have is the conversion of the string to the class.

Not much you can do about it, As you will always have to have some factory which knows how to parse string x into object a,b,c somewhere.

But I would move the logic outside the command handler, which should just accept the command objects. And do the conversion as the strings are read into the application, via a repository (your inputReader or IData?) into which you inject your string parsing factories.

If you allow failover, so that when one repo/factory cant handle an input string it moves onto the next, that will give you further seperation of concerns.

If you use a DI or object serialization/deserialization library in yoir repo/factories (unity,json.net) that will hide some of the reflection/switch statements from your code and make it neater.

Also I would allow for the case where a command cannot be handled. This isnt a bad thing, you should expect only to be able to handle the commands your code 'knows' about and for some other program to deal with the others.

Additionaly, are you sure you need both commandHandler AND command.Execute() it seems to me that you should choose one or the other. If you go with command handlers you cam have one or more per type. The handler has the injected dependancies and the logic from the execute method.

This keeps your handler decoupled, as it only handles a single type of command, you can pull only those types from a queue(IData).

Of course if you keep execute and only pull one type (or set of known types) of object you have the exect same code just arranged differntly, but you can cut out the command handler class, as it just calls execute.

The benefit (if any) of the handler is you can handle the same command type more than one way. Whereas with execute you have to define a new command type with the same data.

example :

class Engine()
{
    IData data; // application database
    IInputReader inputReader;
    Dictionary<string,ICommandHandler> commandHandlers;
    public void Run()
    {
        //todo use a DI framework to inject these types 
        commandHandlers = new Dictionary<string,ICommandHandler>();
        commandHandlers.Add("DisplayData", new DisplayDataCommandHandler(new OutputWriter()) );
        commandHandlers.Add("Build", new BuildDataCommandHandler(new Builder()) );

        foreach(var data in this.data.GetCommands())
        {
            if(commandHandlers.Keys.Contains(data.type))
            {
                var command = inputReader.Read(data.type, data.serializedObject);
                //handle the command
                commandHandlers[data.type].Handle(command);
            }
            else
            {
                //some other program will handle these commands
            }
        }
    }
}

public class InputReaderAndFactory : IInputReader
{
    public ICommand GetCommand(string commandType, string commandJson)
    {
        switch (commandType) 
        {
            case "DisplayData" :
                return JsonConvert.DeserializeObject<DisplayDataCommand>(commandJson);
            case "Build" :
                return JsonConvert.DeserializeObject<BuildCommand>(commandJson);
            default :
                return new UnknownCommand(commandJson);
        }
    }
}

class DisplayDataCommandHandler : ICommandHandler
{
    IOutputWriter outputWriter;
    public DisplayDataCommandHandler(IOutputWriter outputWriter)
    {
        this.outputWriter = outputWriter;
    }

    public void Handle(ICommand command)
    {
        var cmd = command as DisplayDataCommand;
        outputWriter.Print(cmd.Data.ToString());
    }
}
class BuildDataCommandHandler : ICommandHandler
{
    IBuilder builder;
    public BuildDataCommandHandler(IBuilder builder)
    {
        this.builder = builder;
    }

    public void Handle(ICommand command)
    {
        var cmd = command as BuildCommand;
        builder.Build(cmd.Data, cmd.MoreData);
    }
}
class DisplayDataCommand : ICommand
{
    public Data Data {get;set;}
}

class BuildCommand : ICommand
{
    public SomeOtherTypeOfData Data {get;set;}

    public MoreData MoreData {get;set;}
}