Check if `CanExecute` in `RelayCommand.Execute`

[softwareantics]

Overview

Assume a developer wants to call Execute on an ICommand from within another view model. Sure, bad practice but it's something that could happen in a larger project. Doing so will mean that the user forgot to check if the command can actually execute at all.

I vote that we change the RelayCommand.Execute function to check if it can execute before performing it's action.

API breakdown

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool CanExecute(object? parameter)
{
    return this.canExecute?.Invoke() != false;
}

/// <inheritdoc/>
public void Execute(object? parameter)
{
       if (!CanExecute(parameter)
       {
           return;
       }

    this.execute();
}

Usage example

Take the following example where a property is set and NotifyCanExecuteChanged is invoked.

public IRelayCommand BarCommand
{
    get { return this.barCommand ??= new RelayCommand(this.Bar, this.CanBar); }
}

public bool Foo
{
    get { return this.foo; }
    set
    {
        this.SetProperty(ref this.foo, value);
        this.BarCommand.NotifyCanExecuteChanged();
    }
}

private void CanBar()
{
    return someService.CanDoSomething();
}

private void Bar()
{
    // Here, we are not checking if we CanBar
    // But if a user calls BarCommand.Execute() outside of the scope of this view model they may forget to call BarCommand.CanExecute()
    this.someService.DoSomething();
}

With the new changes in place, this will be avoided. Yes, there will be cases where CanExecute is executed twice; but it should theoretically be a quick operation anyways.

Breaking change?

I'm not sure

Alternatives

We can leave the RelayCommand as it is and sometimes forget to call CanExecute where required.

Additional context

https://stackoverflow.com/a/6985124/22134768

Help us help you

Yes, I'd like to be assigned to work on this item