MVVM: Should I check my "CanExecute" method from within my Execute methods?

Question

I understand the use of CanExecute() and Execute(), but I'm wondering about the following scenario:

public class MyViewModel : NotificationObject
{
    public MyViewModel()
    {
        FooCommand = new DelegateCommand(DoFoo, CanDoFoo);
    }

    public Bar MyBar { get; set; }

    public DelegateCommand FooCommand { get; private set; }

    public Boolean CanDoFoo()
    {
        return (MyBar != null)
    }

    public void DoFoo()
    {
        MyBar.BarFunc(); //Potential for a NullReferenceException
    }
}

Basically, a consuming view could decide to call directly into the DoFoo method (obviously breaking the point of the ICommand interface) and cause a NullReferenceException. This may be a bit subjective, but I'm hoping for a "standard" way of doing it.

Do we:

Prevent the possible NullReferenceException by doing a if (MyBar != null) first?
Prevent the possible NullReferenceException by verifying that CanDoFoo() returns true?
Assume that the consuming view is behaving properly and has already verified that it can call into the DoFoo() method?

As a side note, the primary reason I asked this is because when I was writing unit tests I realized that someone could break my ViewModel by calling Execute() methods without calling their CanExecute() counterparts? Obviously in my unit tests I check to see that I can execute the method before doing so, but consuming views might decide to ignore that.

Update: (Scenario 2)

As an expansion to this question, I also want to add in the scenario where the DoFoo() method does not break in terms of exceptions, but could break logically?

public class MyViewModel : NotificationObject
{
    public MyViewModel()
    {
        FooCommand = new DelegateCommand(DoFoo, CanDoFoo);
    }

    public Int32 Age { get; set; }

    public DelegateCommand FooCommand { get; private set; }

    public Boolean CanDoFoo()
    {
        return (Age >= 21)
    }

    public void DoFoo()
    {
        ProvideAlcohal(Age);
    }
}

This second scenario does not actually break (the command can process fine), however, it breaks down logically. So, do we validate the business logic a second time by calling CanDoFoo() or assume the consuming view is behaving? (Remember, this only breaks the business logic).

Basically it boils down to this... Do we put in preventive measures to ensure the consuming view is not shooting itself in the foot by misbehaving?

score 5 · Accepted Answer · answered May 12 '11 at 20:42

5

Any implementation of command calling in WPF or Silverlight will do this, so you don't have to worry about it from the UI system...

But it is a public method. Checking for null is one of the fastest things you can do. It doesn't hurt and it is much more safe since you will throw a null exception without the guard clause.

Semantically, CanExecute may be implemented differently than a null check, so I would just do the null check in the Execute method, not necessarily check CanExecute.

answered May 12 '11 at 20:42

Brian Genisio

47,787
16
124
167

I would also add that checking for null should be a common defensive coding technique anyway. If you truly want an exception on null then check for yourself and handle the exception explicitly. This gives you more control of your exceptions so you can make them more meaningful to your application. – SRM May 12 '11 at 20:52
I expanded my question a bit. I can understand the need to prevent application exceptions, but what about keeping our ViewModel's from breaking down through their business logic? – myermian May 12 '11 at 20:55
It doesn't hurt to check. But the better question is: What do you want to do when `Execute` is called but `CanExecute` is false? Do you want to fail silently? Or do you want to do something to handle that case? It is up to your business logic to determine that. – Brian Genisio May 12 '11 at 21:26
Hmm, I guess it's as I thought... something that's subjective between different people. I was hoping that there was some standard guideline I missed when I was glancing over the MVVM pattern months back. – myermian May 13 '11 at 01:14
Yeah, I think you are not really looking at MVVM anymore, and just your business logic. – Brian Genisio May 13 '11 at 01:22

MVVM: Should I check my "CanExecute" method from within my Execute methods?

1 Answers1