9

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);
Matthew
  • 28,056
  • 26
  • 104
  • 170

3 Answers3

8

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.

themaestro
  • 13,750
  • 20
  • 56
  • 75
  • 2
    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. – JerKimball Aug 04 '11 at 16:43
5

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.

Stu Mackellar
  • 11,510
  • 1
  • 38
  • 59
  • 1
    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. – themaestro Aug 04 '11 at 14:53
  • 2
    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. – Stu Mackellar Aug 04 '11 at 15:13
  • 1
    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. – themaestro Aug 04 '11 at 16:22
  • 2
    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. – Stu Mackellar Aug 04 '11 at 16:32
  • 1
    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. – JerKimball Aug 04 '11 at 16:41
  • 1
    @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? – Matthew Aug 04 '11 at 18:25
  • 1
    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. – Matthew Aug 04 '11 at 18:28
  • 1
    @Matthew - 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. – JerKimball Aug 07 '11 at 04:24
2

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

Ray
  • 45,695
  • 27
  • 126
  • 169