Design pattern - Client Server - Command pattern

Question

I have a bunch of commands that I need to batch from the client and execute at the server. These commands are of different types and the contract for the command and the corresponding return types is shared between the client and server via a library.

Client side code is as follows-

var client = new ClientSDK();
client.Add(new Command1());
client.Add(new Command2());
client.Add(new Command3());

// Execute transmits all the commands to the server 
var results = client.Execute();

Server code -

List<CommandResult> Execute(List<CommandBase> commands)
{
   List<CommandResult>  results = new List<CommandResult>();
   foreach(CommandBase command in commands)
   {
       if(command.GetType == Command1)
       {
           results.Add(new Command1Executor(command).Execute())
       }
       else if(command.GetType == Command2)
       {
           results.Add(new Command1Executor(command).Execute())
       }
       else if(command.GetType == Command3)
       {
           results.Add(new Command3Executor(command).Execute())
       }    
       ..................     
   }
}

For each command there exists a unique execute function, that cannot be exposed as part of the client SDK. How do I make design changes such that I can get rid of the massive if/else block? There are tons of commands to be supported. I tried applying the command pattern as suggested here - using the command and factory design patterns for executing queued jobs but this requires each command to implement the ICommand interface, which is not possible

Is there a better way to design this?

Answer 1

Basically, you need to map CommandNExcecutor type to CommandN type.

1) Use dictionary. This is easiest way:

private static readonly Dictionary<Type, Type> map = new Dictionary<Type, Type>
{
    {  typeof(Command1), typeof(Command1Executor) },
    {  typeof(Command2), typeof(Command2Executor) },
    ...
};

List<CommandResult> Execute(List<CommandBase> commands)
{
    return commands
        .Select(command =>
        {
            var executor = Activator.CreateInstance(map[command.GetType], command);
            return executor.Execute();
        })
        .ToList();
}

2) Use metadata (attributes). This fits plugin-based scenarios, when command types can be added dynamically, without rebuilding core functional. It could be your own implementation, or existing DI-container implementation (many of them expose metadata APIs).

[AttributeUsage(AttributeTargets.Class)]
public sealed class CommandExecutorAttribute : Attribute
{
    public CommandExecutorAttribute(Type commandType)
    {
        CommandType = commandType;
    }

    public Type CommandType { get; }

    // ...
}

[CommandExecutor(typeof(Command1))]
public sealed class Command1Executor : ICommandExecutor
{
    // ...
}

List<CommandResult> Execute(List<CommandBase> commands)
{
    return commands
        .Select(command =>
        {
            // obtain executor types somehow, e.g. using DI-container or
            // using reflection;
            // inspect custom attribute, which matches command type
            var executorType = ....

            var executor = Activator.CreateInstance(executorType , command);
            return executor.Execute();
        })
        .ToList();
}

UPDATE.

If you want to avoid reflection, in first case just replace type parameter for value in dictionary to Func<CommandBase, ICommandExecutor>:

private static readonly Dictionary<Type, Func<ICommandExecutor>> map = new Dictionary<Type, Func<ICommandExecutor>>
    {
        {  typeof(Command1), command => new Command1Executor(command) },
        {  typeof(Command2), command => new Command2Executor(command) },
        ...
    };

This will allow you to create executor through delegate instead of reflection:

var executor = map[command.GetType](command);

The second case can't avoid reflection completely, since you need to get executor types somehow. But it can be leaded to case 1 (with dictionary).

Make the lazy map:

private static readonly Lazy<Dictionary<Type, ConstructorInfo>> map = ...

Then, on Lazy<T> initialization, do all reflection work. Since this is static Lazy<T>, you will do this once per app domain. Calling ConstructorInfo is fast enough. Hence, you'll get performance hit only once, when handling first command.

Another options (they all assume Lazy<Dictionary>) for case 2 are:

• instead of reflecting ConstructorInfo, build Expression<Func<CommandBase, ICommandExecutor>>s using ConstructorInfo, compile them and put delegates into dictionary - this will be the same as delegates for case 1, but with dynamically supported command types;
• use DI-container, which emits IL to construct dependencies (AFAIK, NInject does this);
• emit IL yourself (IMO, this will totally re-invent the wheel).

And finally, solve this the easiest way, then measure performance, then consider more complex way. Avoid premature optimization. I don't know anything about you commands nature, but I suspect, that command execution is much longer, than reflecting something (of course, there's a chance, that I'm wrong).

Hope this helps.

Answer 2

Try using the strategy pattern. A simple solution is below:

public class CommandStrategy 
{
    private static Dictionary<CommandTypes, Action<CommandStrategy>> strategy;

    public CommandStrategy()
    {
        strategy = new Dictionary<CommandTypes, Action<CommandStrategy>>();
        strategy.Add(CommandTypes.Command1, one => new Command1Executor().Execute());
        strategy.Add(CommandTypes.Command2, two => new Command2Executor().Execute());
        strategy.Add(CommandTypes.Command3, two => new Command3Executor().Execute());
    }

    public void Execute(CommandTypes type)
    {
        strategy[type].Invoke(this);
    }
}

The final execution might look like this:

CommandStrategy strategy = new CommandStrategy();

List<CommandBase> commands = new List<CommandBase>(){
                  new Command1(), new Command2(), new Command3() };

foreach (var item in commands)
{
   CommandTypes type = (CommandTypes)item;
   strategy.Execute(type);
}