ICommand - Should I call CanExecute in Execute?

Question

Given that System.Windows.Input.ICommand as 2 primary methods:

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

I expect CanExecute(...) to be called in the Command-supported frameworks before Execute(...) is called.

Internally to my Command Implementation, however, is there any reason to add the CanExecute(...) call inside my Execute(...) implementation?

e.g.:

public void Execute(object parameters){
  if(!CanExecute(parameters)) throw new ApplicationException("...");
  /** Execute implementation **/
}

This becomes relevant in my testing, as I may mock out some interfaces to support CanExecute, and have to do the same mocks when testing Execute.

Any design thoughts on this?

Answer 1

Programmers are notoriously lazy and they will call Execute without calling CanExecute first.

The ICommand interface is more often used with the WPF binding framework however it is an extremely robust and useful pattern that can be used elsewhere.

I call CanExecute immediately from the Execute method to validate the state of the object. It helps reduce duplicate logic and enforces the use of the CanExecute method (why go through all the effort of whether they can call a method and not bother enforcing it?). I don't see a problem with calling CanExecute lots of times because it should be a quick operation anyway.

I do however always document the results of calling an Execute method if the CanExecute method returns false, so that the consumer knows the consequences.

Answer 2

I would not be as optimistic as others about adding a call to CanExecute into Execute implementation. What if your CanExecute execution takes a very long time to complete? This would mean that in real-life your user will wait twice that long - once when CanExecute is called by environment, and then when it is called by you.

You could possibly add some flags to check whether CanExecute has already been called, but be careful to keep them always up to command state not to miss or perform unwanted CanExecute call when the state has changed.

Answer 3

I would go one way or the other, but not both.

If you expect the user to call CanExecute, then don't call it in Execute. You've already set that expectation in your interface and now you have a contract with all the developers that implies this sort of interaction with ICommand.

However, if you're worried about developers not utilizing it properly (as you could rightfully be), then I would suggest removing it from the interface completely, and making it an implementation concern.

Example:

interface Command {
    void Execute(object parameters);    
}

class CommandImpl: ICommand {
    public void Execute(object parameters){
        if(!CanExecute(parameters)) throw new ApplicationException("...");
        /** Execute implementation **/
    }
    private bool CanExecute(object parameters){
        //do your check here
    }
}

This way, you're contract (interface) is clear and concise, and you won't be confused about whether or not CanExecute is getting called twice.

However, if you're actually stuck with the interface because you don't control it, another solution could be to store the results and check it like this:

interface Command {
    void Execute(object parameters);    
    bool CanExecute(object parameters);
}

class CommandImpl: ICommand {

    private IDictionary<object, bool> ParametersChecked {get; set;}

    public void Execute(object parameters){
        if(!CanExecute(parameters)) throw new ApplicationException("...");
        /** Execute implementation **/
    }

    public bool CanExecute(object parameters){
        if (ParametersChecked.ContainsKey(parameters))
            return ParametersChecked[parameters];
        var result = ... // your check here

        //method to check the store and add or replace if necessary
        AddResultsToParametersChecked(parameters, result); 
    }
}