70

My question is related to the command pattern, where we have the following abstraction (C# code) :

public interface ICommand
{
    void Execute();
}

Let's take a simple concrete command, which aims to delete an entity from our application. A Person instance, for example.

I'll have a DeletePersonCommand, which implements ICommand. This command needs the Person to delete as a parameter, in order to delete it when Execute method is called.

What is the best way to manage parametrized commands ? How to pass parameters to commands, before executing them ?

Stelios Adamantidis
  • 1,866
  • 21
  • 36
Romain Verdier
  • 12,833
  • 7
  • 57
  • 77
  • 10
    I know this question dates over four year back, but Juanma and bloparod actually give the correct answer: make `ICommand` generic (`ICommand`). The given `TArgs` encapsulates all arguments (it becomes a [Parameter Object](http://c2.com/cgi/wiki?ParameterObject)). You will have to create two objects per command: one for the message; one for the behavior. This sounds awkward at first, but when you get it, you'll never look back. [This article](http://www.cuttingedge.it/blogs/steven/pivot/entry.php?id=91) describes this model in detail. A must read for everybody who read this question. – Steven Dec 28 '12 at 23:43
  • 1
    @Steven thanks for the link to your blog post. Perhaps it would be good if you could clarify how the approach you describe in it fits with the question here given that, by your own admission, you "don't consider [it] the Command Pattern". One could get the notion that your comment is simply self-promotion. – ardila Apr 25 '16 at 10:49

13 Answers13

73

You'll need to associate the parameters with the command object, either by constructor or setter injection (or equivalent). Perhaps something like this:

public class DeletePersonCommand: ICommand
{
     private Person personToDelete;
     public DeletePersonCommand(Person personToDelete)
     {
         this.personToDelete = personToDelete;
     }

     public void Execute()
     {
        doSomethingWith(personToDelete);
     }
}
Brad
  • 11,934
  • 4
  • 45
  • 73
Blair Conrad
  • 233,004
  • 25
  • 132
  • 111
  • 3
    Exactly what I'd do. For anything that isn't known when the command is constructed, I'd pass in the interface to a service that gets the object when the command is executed. That could be a delegate or lambda expession or another object. – Hamish Smith Sep 20 '08 at 00:11
  • 1
    This is a poor solution because the container is tightly coupled with the Person, instead that coupling should be broken using some kind of Parameter object, which contains the dependencies. Tell Don't Ask is the prime rule here. – Martin of Hessle Mar 19 '19 at 11:13
  • @Blair Conrad I wonder that what if we change receiver method. According to open/closed principle, changing Command's execute method will be ok? – uzay95 Nov 13 '19 at 01:09
  • Other than coupling, I am not even sure if this is thread safe. Especially in the UI context where multiple threads might want to access the same person. Between saving the copy of the "personToDelete" and actual execution of the command, the value might change. – Mayur Ekbote Sep 08 '22 at 17:30
  • @MayurEkbote if you worrie about that, then passing in contructor or into method does not change your worries. its a object so it will be passed as a reference. – Niels Lucas Aug 23 '23 at 14:23
  • @MartinofHessle its not, a command should be a container with its context. A command is just a object that is a method. Nothing more. Its not a service with methods you want to reuse for multiple contexts. A good example for why you want the Execute function be plain as simple is for when you have a List. Your object can be created however you like. All the context (data) is passed in the contructor. Add to the List. If you loop the List it can execute all kinds of different stuff no matter the context because the Execute() does not care about the context. – Niels Lucas Aug 23 '23 at 14:28
23

Passing the data in via a constructor or setter works, but requires the creator of the command to know the data the command needs...

The "context" idea is really good, and I was working on (an internal) framework that leveraged it a while back.

If you set up your controller (UI components that interact with the user, CLI interpreting user commands, servlet interpreting incoming parameters and session data, etc) to provide named access to the available data, commands can directly ask for the data they want.

I really like the separation a setup like this allows. Think about layering as follows:

User Interface (GUI controls, CLI, etc)
    |
[syncs with/gets data]
    V
Controller / Presentation Model
    |                    ^
[executes]               |
    V                    |
Commands --------> [gets data by name]
    |
[updates]
    V
Domain Model

If you do this "right", the same commands and presentation model can be used with any type of user interface.

Taking this a step further, the "controller" in the above is pretty generic. The UI controls only need to know the name of the command they'll invoke -- they (or the controller) don't need to have any knowledge of how to create that command or what data that command needs. That's the real advantage here.

For example, you could hold the name of the command to execute in a Map. Whenever the component is "triggered" (usually an actionPerformed), the controller looks up the command name, instantiates it, calls execute, and pushes it on the undo stack (if you use one).

Scott Stanchfield
  • 29,742
  • 9
  • 47
  • 65
12

There are some options:

You could pass parameters by properties or constructor.

Other option could be:

interface ICommand<T>
{
    void Execute(T args);
}

And encapsulate all command parameters in a value object.

Steven
  • 166,672
  • 24
  • 332
  • 435
Juanma
  • 3,961
  • 3
  • 27
  • 26
  • 13
    The problem with the code above is that different commands (e.g. CreateSomeThingCommand and DeleteSomethingCommand) might require different parameters and cant be executet the same way anymore (thinking of IEnumerable .Execute() call). Command pattern is meant to be used to separate definition from execution... if you pass parameters at the execution time you are changing/controlling the behaviour of the command at execution time instead of defintion time. – Beachwalker Sep 12 '12 at 11:42
  • By the way: I think you meant void Execute(T args) instead of Execute(T args>, because T is already defined at ICommand, a second one at function/method level is useless. You could create something like the following, too: interface ICommand { void Execute(T1 t1, T2 t2); } (which makes more sene) or interface ICommand { void Execute(T2 t2); // using T1 anywhere else } – Beachwalker Sep 12 '12 at 12:23
8

My implementation would be this (using the ICommand proposed by Juanma):

public class DeletePersonCommand: ICommand<Person>
{
    public DeletePersonCommand(IPersonService personService)
    {  
        this.personService = personService;
    }

    public void Execute(Person person)
    {
        this.personService.DeletePerson(person);
    }
}

IPersonService could be an IPersonRepository, it depends in what "layer" your command is.

bloparod
  • 1,716
  • 1
  • 14
  • 17
6

Pass the person when you create the command object:

ICommand command = new DeletePersonCommand(person);

so that when you execute the command, it already knows everything that it needs to know.

class DeletePersonCommand : ICommand
{
   private Person person;
   public DeletePersonCommand(Person person)
   {
      this.person = person;
   }

   public void Execute()
   {
      RealDelete(person);
   }
}
jop
  • 82,837
  • 10
  • 55
  • 52
6

In this case, what we've done with our Command objects is to create a Context object which is essentially a map. The map contains name value pairs where the keys are constants and the values are parameters that are used by the Command implementations. Especially useful if you have a Chain of Commands where later commands depend on context changes from earlier commands.

So the actual method becomes

void execute(Context ctx);
user12786
  • 752
  • 3
  • 3
5

In the constructor and stored as fields.

You will also want to eventually make your ICommands serializable for the undo stack or file persistence.

Frank Krueger
  • 69,552
  • 46
  • 163
  • 208
5

Already mentioned code from Blair Conrad(don't know how to tag him) works perfectly fine if you know what person you want to delete when you instantiate the class and his method would suffice.But,if you don't know who you gonna delete until you press the button you can instantiate the command using a method reference that returns the person.

   class DeletePersonCommand implements ICommand
{
     private Supplier<Person> personSupplier;

     public DeletePersonCommand(Supplier<Person> personSupplier)
     {
         this.personSupplier = personSupplier;
     }

     public void Execute()
     {
        personSupplier.get().delete();
     }
}

That way when the command is executed the supplier fetches the person you want to delete,doing so at the point of execution. Up until that time the command had no information of who to delete.

Usefull link on the supplier.

NOTE:code writen in java . Someone with c# knowledge can tune that.

Kostas Thanasis
  • 368
  • 4
  • 11
3

Based on the pattern in C#/WPF the ICommand Interface (System.Windows.Input.ICommand) is defined to take an object as a parameter on the Execute, as well as the CanExecute method.

interface ICommand
            {
                bool CanExecute(object parameter);
                void Execute(object parameter);
            }

This allows you to define your command as a static public field which is an instance of your custom command object that implements ICommand.

public static ICommand DeleteCommand = new DeleteCommandInstance();

In this way the relevant object, in your case a person, is passed in when execute is called. The Execute method can then cast the object and call the Delete() method.

public void Execute(object parameter)
            {
                person target = (person)parameter;
                target.Delete();
            } 
TheZenker
  • 1,710
  • 1
  • 16
  • 29
  • 3
    The way the "pattern" is implemented this way is nothing more than a "special" delegate with validation (CanExecute). I think, this looses the real functionality the pattern is made for... decoupling of definition and execution of a command. Passing in params would/could change the way of execution. This way the definition of the command is taken from the constructor of the command to the parameter creation time istead. (I know M$ used this for GUI purposes but I don't think this should be the common approach for implementing the command pattern.) – Beachwalker Sep 12 '12 at 11:50
2

You should create a CommandArgs object to contain the parameters you want to use. Inject the CommandArgs object using the constructor of the Command object.

David Robbins
  • 9,996
  • 7
  • 51
  • 82
0

DeletePersonCommand can have parameter in its constructor or methods . DeletePersonCommand will have the Execute() and in the execute can check attribute that will be passed by Getter/Setter previously the call of the Execute().

Patrick Desjardins
  • 136,852
  • 88
  • 292
  • 341
0

I would add any necessary arguments to the constructor of DeletePersonCommand. Then, when Execute() is called, those parameters passed into the object at construction time are used.

Matt Dillard
  • 14,677
  • 7
  • 51
  • 61
-5

Have "Person" implement some sort of IDeletable interface, then make the command take whatever base class or interface your entities use. That way, you can make a DeleteCommand, which tries to cast the entity to an IDeletable, and if that works, call .Delete

public class DeleteCommand : ICommand
{
   public void Execute(Entity entity)
   {
      IDeletable del = entity as IDeletable;
      if (del != null) del.Delete();
   }
}
Joel Martinez
  • 46,929
  • 26
  • 130
  • 185
  • 1
    I don't think this works - the whole point of ICommand is that every subclass overrides Execute() _exactly_. This solution requires the caller of Execute() to know more details about the type of command being called. – Matt Dillard Sep 19 '08 at 19:51
  • I agree with Matt. That DeleteCommand class wouldn't even compile, anyhow, since it doesn't implement void Execute() as required by the ICommand interface – chadmyers Sep 19 '08 at 19:52
  • Using dependency injection, you still have to know everything about the type of command, because you have to new it up! at least this way your code can be generic if you only deal with "Entity". The original response includes info about changing ICommand to include the base class/interface. – Joel Martinez Sep 19 '08 at 19:57