using the command and factory design patterns for executing queued jobs
Asked Answered
B

3

8

I have a list of jobs queued in the database which I need to read from database and execute them in parallel using threading and I have a list of command classes to execute each of those jobs all implementing a common interface (command pattern). but when I retrieve the pending jobs from the database, I will need to instantiate the right command object for each job something like this (in a factory class)

ICommand command;
switch (jobCode)
{
  case "A":
     command = new CommandA();
     break;
  case "B":
     command = new CommandB();
     break;
  case "C":
     command = new CommandC();
     break;
}

command.Execute();

Is there a better way to create the right command object without using a big switch statement like above? OR is there any other pattern for executing the queued jobs?

Solution: solved like this (based on the selected answer). This will do lazy instantiation of the command objects.

public class CommandFactory
{
    private readonly IDictionary<string, Func<ICommand>> _commands;

    public CommandFactory()
    {
        _commands = new Dictionary<string, Func<ICommand>>
                        {
                            {"A", () => new CommandA()},
                            {"B", () => new CommandB()},
                            {"C", () => new CommandC()}
                        };
    }

    public ICommand GetCommand(string jobKey)
    {
        Func<ICommand> command;
        _commands.TryGetValue(jobKey.ToUpper(), out command);
        return command();
    }
}    

Client: 

        var factory = new CommandFactory();
        var command = factory.GetCommand(jobKey);
        command.Execute();
Baldheaded answered 9/1, 2012 at 12:21 Comment(1)
This seems flawed considering all of your commands will have to be in your factory.Twelvetone
I
14

Most C# command pattern implementations more or less the same as a Java implementation. These implementations usually use a ICommand interface:

public interface ICommand
{
    void Execute();
}

and then all command classes are forced to implement the interface. I have no issues with this solution, but personally I don't like creating too many classes and I prefer to use .NET delegates instead (there are no delegates in Java). The Action delegate usually does the trick if only need one method reference:

public class Prog
{
    public Prog()
    {
        var factory = new CommandFactory();
        factory.Register("A", () => new A().DoA);            
        factory.Register("B", () => new B().DoB);
        factory.Register("C", DoStuff);

        factory.Execute("A");
    }

  public static void DoStuff()
    {
    }
}

public class CommandFactory
{
    private readonly IDictionary<string, Action> _commands;       

    public void Register(string commandName, Action action)
    {
    _commands.Add(commandName, action); 
    }

    public Action GetCommand(string commandName)
    {
        _commands[commandName];
    }

    public void Execute(string commandName)
    {
        GetCommand(commandName)();
    }
}
public class A
{
    public void DoA()
    {
    }
}

public class B
{
    public void DoB()
    {
    }
}

If your command interface needs more than one methods like:

public interface ICommand
{
    void Execute();
    void Undo();
}

You can use a wrapper class like this:

public class Command
{
    public Command(Action execute, Action undo)
    {
        Execute = execute;
        Undo = undo;
    }

    public Action Execute { get; protected set; }
    public Action Undo { get; protected set; }
}

or (it doesn't matter which one)

public class Command 
{
    private readonly Action _execute;
    private readonly Action _undo;

    public Command(Action execute, Action undo)
    {
        _execute = execute;
        _undo = undo;
    }

    public void Execute()
    {
        _execute();
    }

    public void Undo()
    { 
        _undo();
    }
}

(this one may even implement ICommand if you have legacy stuff using it already. If you use the interface the factory should use the interface instead of the Command class)

With a wrapper like this you are not forced to create a command class for each action you want to support. The following example demonstrates how you can use the wrapper class:

public class Prog2
{
    public Prog2()
    {
        var factory = new CommandFactory2();
        factory.Register("A", new Lazy<Command>(
            ()=>
                {
                    var a = new A();
                    return new Command(a.DoA, a.UndoA);
                }));

        factory.Register("B", new Lazy<Command>(
           () =>
           {
               var c = new B();
               return new Command(c.DoB, c.DoB);
           }));

        factory.Register("C", new Lazy<Command>(
            () => new Command(DoStuff, UndoStuff)));

        factory.Execute("A");
    }

    public static void DoStuff()
    {
    }

    public static void UndoStuff()
    {
    }
}

public class CommandFactory2
{
    private readonly IDictionary<string, Lazy<Command>> _commands;

    public void Register(string commandName, Lazy<Command> lazyCommand)
    {
        _commands.Add(commandName, lazyCommand);
    }

    public void Register(string commandName, Action execute, Action undo)
    {
        _commands.Add(commandName, new Lazy<Command>(() => new Command(execute, undo)));
    }

    public Command GetCommand(string commandName)
    {
        return _commands[commandName].Value;
    }

    public void Execute(string commandName)
    {
        GetCommand(commandName).Execute();
    }

    public void Undo(string commandName)
    {
        GetCommand(commandName).Undo();
    }
}


public class A
{
    public void DoA()
    {
    }

    public void UndoA()
    {
    }
}

public class B
{
    public void DoB()
    {
    }

    public void UndoB()
    {
    }
}

As you can see there is no need to implement the interface even if you have more than one method (Execute, Undo, etc). Please note that the Execute and Undo methods may belong to different classes. You are free to structure your code the way it feels more natural and still can use the command pattern.

Inextinguishable answered 9/1, 2012 at 12:49 Comment(14)
your provided example lacks design flexibility and reusability, your provided example is pushing user to create a more like hard coded class..i preffer @Slade's example it's better one.Cordiecordier
Please explain how it lacks design flexibility and reusability? You can add any method to the dictionary which satisfies the delegate signature. What do you mean by "more like hard coded class"?Inextinguishable
factory.Register("C" Is one of those hard coding which is a big flaw in the design. So do not hard code.Showery
I don't understand even with Slade's example you have to specify the keys like factory.RegisterCommand<SomeCommand>("C"). Let me point out that my sample allows won't force you to create an instance, plus you can register methods of non empty constructor classes as well.Inextinguishable
Thanks. how can this be improved for lazy instantiationBaldheaded
I added the lazy implementation too. You need a dictionary of string and Lazy<Action>. I don't think my solution is less flexible: you are not limited to parameterless command classes and you don't have to implement an interface. You can even register static methods if you like.Inextinguishable
it helped. thanks. can this be changed just to get an instance of the command object alone, instead of getting a handle to its execute method? because I think factory should only be concerned with creating objects, not executing them (SRP).Baldheaded
The best thing is that since you use functional style you can even use closures; you can instantiate the target class only once: codethinked.com/c-closures-explained I might write an example later which demonstrates this.Inextinguishable
I added GetCommand which will give you back the Action delegate by name.Inextinguishable
I mean to say can the GetCommand return a command object implementing "ICommand"? in your example it returns a delegate to the "Execute" method.Baldheaded
This implementation forces the client code to know about the command internals because you provide the action implementation when registering the command. Command pattern is to decouple consumer from provider.Dappled
@Dappled the client only needs to know the factory and the command name. The registration of names and commands is required but the client doesn't need to know about it. IoC containers which use keyed or named registrations work the same way: you register the type for the name or key and then the lookup happens based on key or name in the client. Please provide an alternative answer where you explain what you mean.Inextinguishable
@JenoLaszlo In your example, you register the command before calling the execute method inside the same method. Thus the client knows the command implementation (the register step). I was thinking, if the command factory is passed to the client as dependency, with the commands already registered, the problem is solved.Dappled
@Dappled but the factory invocation is just an example how the command can be invoked. The question was about command pattern. It a real world scenario you would inject the factory, or the delegates or the command interfaces into the client class which would hide the registration details. I assumed this part goes without saying.Inextinguishable
C
4

You could use a Dictionary to map the letter/character to the relevant ICommand implementation. Something like:

public class CommandFactory
{
    private readonly Dictionary<string, ICommand> mCommands = new Dictionary<string,ICommand>(StringComparer.OrdinalIgnoreCase);

    public void RegisterCommand<TCommand>(string commandKey) where TCommand : ICommand, new()
    {
        // Instantiate the command
        ICommand command = new TCommand();

        // Add to the collection
        mCommands.Add(commandKey, command);
    }

    public void ExecuteCommand(string commandKey)
    {
        // See if the command exists
        ICommand command;
        if (!mCommands.TryGetValue(commandKey, out command))
        {
            // TODO: Handle invalid command key
        }

        // Execute the command
        command.Execute();
    }
}

Using this, you can register command types and map them to string-based keys and allow them to be instantiated and executed more generically. You could improve performance by only instantiating the command types when they are first used.

EDIT

In answer to your comment, to instantiate only when executing, you could do something like:

public class CommandDetails<T> where T : ICommand, new()
{
    private ICommand mCommand;

    public ICommand GetCommand()
    {
        if (/* Determine if the command has been instantiated */)
        {
            // Instantiate the command
            mCommand = new T();
        }

        return mCommand;
    }
}

public void ExecuteCommand(...)
{
    // See if the command exists
    CommandDetails details;
    // ...

    // Get the command
    // Note: If we haven't got the command yet, this will instantiate it for us.
    ICommand command = details.GetCommand();

    // ...
}
Cowbell answered 9/1, 2012 at 12:32 Comment(3)
Thanks. this is a classic example of the command pattern, storing all command objects in a collection in a container class, but how can this be improved to instantiate them only when they are used?Baldheaded
but how can I register the command (add the command to the dictionary) in the first place without instantiating it? one option is to use delegates as specified in another answer. interested to know if there are any more solutions.Baldheaded
In my extended answer, the registration would work in pretty much the same way, except you would store CommandDetails in the Dictionary. To get around the lack of common type parameters, you could have the CommandDetails implement a generic ICommandDetails interface, which would have only a method with signature: ICommand GetCommand();.Cowbell
A
1

You could consider asking your job to provide it's own ICommand:

interface IJob 
{
  ICommand Command { get; }
}

public class JobA : IJob
{
  private readonly ICommand _command = new CommandA();
  public ICommand Command { get { return _command; } }
}

Then rather than switching on the jobCode, you could just do:

job.Command.Execute();
Autobiographical answered 9/1, 2012 at 12:30 Comment(4)
not sure if this will help. instead of switching to create a command object, I need a switch to create the right job object using this.Baldheaded
@Baldheaded I assumed your jobCode would come from a job - at the point where the job is instantiated you know which type of job you want and hence which command also.Sexdecillion
The point again comes to the discussion which to create instead of ICommand now he needs to look into IJob :)Cordiecordier
Did you find a better design pattern for this issue ? ThanksHaviland

© 2022 - 2024 — McMap. All rights reserved.