Should I check an ICommand's CanExecute method before calling Execute from procedural code?

Question

When using ICommands in XAML, WPF uses the CanExecute method to enable or disable controls associated with the command. But what if I am calling Execute from procedural code? Should I first check CanExecute to make sure that the command can execute, or should Execute take care of this check for me?

In other words, should I do this:

if (someCommand.CanExecute(parameter, target))
    someCommand.Execute(parameter, target);

Or just this:

someCommand.Execute(parameter, target);

Unmanly

Why not make this part of Execute()? – Tripitaka

Answer 1

Good style would dictate that you should do the former, check CanExecute first. This will enforce proper decomposition and a consistency in implementation. Also, in the event you ever do want to use this command bound to a button, it will work as expected.

Emulate

Contractually, this feels "righter". I'd also add that if the condition you are checking relies in any way on state extrinsic to the command (and hence modifiable by another thread, etc), you'd want to lock around the CanExecute/Execute sequence. – Winne

Answer 2

You should just call Execute and let the command implementation handle validation. CanExecute is mainly provided for UI state bindings.

Except for very simple single-threaded scenarios even if you do call CanExecute first there could easily be a race condition whereby the command validity changes between the CanExecute and the Execute calls, rendering the call to CanExecute pointless.

Reset

I don't see how a race condition would behave differently depending on calling CanExecute then Execute versus just calling Execute. Those calls will be execute sequentially regardless. If you're referring to a background thread, it's just as possible that there is a context switch in the middle of Execute as there being one between CanExecute and Execute. – Emulate

As you've just pointed out they won't necessarily be executed sequentially and, even if they are, what's to stop another thread changing the commands's underlying data or state between those two sequential calls? Whereas the command implementation knows the context it's operating in and can synchronise access to shared data atomically if it needs to. It's also best placed to determine if the command is valid at the point of execution. – Reset

The command's underlying data can change at any point in the execution without regard to whether or not you check within the Execute call or the CanExecute call. I do, however, see the case for synchronizing access if necessary. I would argue though, that this is a minority of cases, and that while necessary in a niche prone to race conditions, in general design it is better stylistically to use CanExectue. – Emulate

The data can't change if access to it is synchronised. My point is that it's only the command implementation that's in a position to know if this is necessary. Also, there is no such thing as a "niche prone to race conditions": either there's concurrent access to data, in which case it needs to be protected, or there's not. You seem to be arguing that because race conditions of this type occur rarely that they can be disregarded. This is a dangerous attitude - if there are millions of users running the program the likelihood of an problem occurring tends towards inevitability. – Reset

You both make valid points: it is possible, depending on conditions, for a race condition to arise from the CanExecute/Execute gap, but forcing the constraints checking into the Execute method and never checking CanExecute seems like a contractual violation to me. I'd argue from a purely aesthetic basis that CanExecute should contain your precondition checking, and the whole CanExecute/Execute sequence should be contained in a lock when invoked procedurally. I'd argue this mainly because contractually, it's the responsibility of CanExecute make that determination, hence the name. – Winne

@JerKimball: You could argue that CanExecute is only intended to be used to provide information (to be used by frameworks like WPF) about whether a Command can be executed, and that Executed is not able to depend on CanExecute being checked unless Execute itself calls CanExecute. At the very least, there is no guarantee that a programmer will check CanExecute before calling Execute. What do you think about this argument? – Unmanly

Or as a third alternative: Why not do both (call CanExecute and also validate in Execute)? This way, whoever wrote the command does not have to rely on whoever is using the command to check CanExecute first. At the same time, whoever is using the command can be certain the the code will work as expected without knowing that Executed also validates. – Unmanly

@Unmanly - hmm...you do make a point, and the general concept of "Commands" in other frameworks/libraries don't make the distinction between CanExecute/Execute...I don't know, I'm on the fence: semantics-wise, it seems right to separate the two, and it does make sense to separate the two if the cost of executing the command is prohibitive compared to the cost of "just checking", but practically speaking, I agree that most folks won't remember to call both. – Winne

Answer 3

You need to call CanExecute first, there's nothing that says that classes that implement ICommand check their CanExecute in their Execute method.

Bambibambie