Should CanExecute work forever or just when property is changed?-CodePudding

I am working on a calculator. My view contains 16 Buttons and 1 TextBox:

TextBox's Text property is bound to the UserInput property in ViewModel:

<TextBox x:Name="InputTextBox"
         Grid.Row="0"
         Background="#ECDBBA"
         Foreground="#191919"
         FontSize="30"
         Text="{Binding UserInput, UpdateSourceTrigger=PropertyChanged}"
         IsReadOnly="True"
         VerticalContentAlignment="Center"/>

public class ViewModel : INotifyPropertyChanged
{
    public AddTextCommand AddTextCmnd { get; set; }
    public ClearTextCommand ClearTextCmnd { get; set; }
    public ShowResultCommand ShowResultCmnd { get; set; }

    private string _userInput = "0";
    public string UserInput
    {
        get { return _userInput; }
        set
        {
            if (UserInput != value)
            {
                _userInput = value;
                OnPropertyChanged("UserInput");                                    
            }
            
        }
        
    }

    public ViewModel()
    {
        AddTextCmnd = new AddTextCommand(this);
        ClearTextCmnd = new ClearTextCommand(this);
        ShowResultCmnd = new ShowResultCommand(this);
    }

    public event PropertyChangedEventHandler PropertyChanged;
    public void OnPropertyChanged(string propertyName = "")
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));            
    }
}

I have 3 commands: One to add text to TextBox, another to show the result, and a last one to clear the TextBox. AC and "=" buttons (see screenshot) are bound to ClearTextCommand and ShowResultCommand.

public class ShowResultCommand : ICommand 
    {
        private ViewModel _viewModel;

        public ShowResultCommand(ViewModel viewModel)
        {
            _viewModel = viewModel;
        }


        public bool CanExecute(object parameter)
        {
            if (_viewModel.UserInput != "0")
                return true;

            else
                return false;            
        }

        public void Execute(object parameter)
        {
            DataTable calculator = new DataTable();
            _viewModel.UserInput = calculator.Compute(_viewModel.UserInput, null).ToString();
        }

        public event EventHandler CanExecuteChanged
        {
            add
            {
                CommandManager.RequerySuggested  = value;
            }
            remove
            {
                CommandManager.RequerySuggested -= value;
            }
        }
    }
}

public class ClearTextCommand : ICommand
{
    private ViewModel _viewModel;

    public ClearTextCommand(ViewModel viewModel)
    {
        _viewModel = viewModel;
    }

    public bool CanExecute(object parameter)
    {
        if (_viewModel.UserInput != "0")
            return true;

        else
            return false;
    }

    public void Execute(object parameter)
    {
        _viewModel.UserInput = "0";
    }

    public event EventHandler CanExecuteChanged
    {
        add
        {
            CommandManager.RequerySuggested  = value;
        }
        remove
        {
            CommandManager.RequerySuggested -= value;
        }
    }
}

The program works well, but 2 commands share a problem. I don't know if it is my ignorance or not, but I didn't find anything in Google. I thought that my CanExecute methods work only when UserInput changed (property changed), but I put a breakpoint on the CanExecute methods of my commands and saw that they work forever in debug, like once UserInput is changed, ShowResultCommand and ClearTextCommand's CanExecute methods work forever, they are called again and again in a loop. I am wondering if they should work this way? Shouldn't they be called just when UserInput is changed? This doesn't cause any error in my program, but I think there is something wrong with my commands.

So basically question is:

Should CanExecute work in a loop while my app is running or should it work when a property related to the method is changed? If it should work in a loop, then everything is fine, but if not what is wrong with my commands?

CodePudding user response：

I thought that my CanExecute methods work only when UserInput changed(property changed).

Yes and no. Your commands delegate the CanExecuteChanged event to the RequerySuggested event of CommandManager. This is a common approach. The CommandManager is a WPF framework type responsible for:

Provides command related utility methods that register CommandBinding and InputBinding objects for class owners and commands, add and remove command event handlers, and provides services for querying the status of a command.

The RequerySuggested event is only raised under a few not well documented circumstances:

The CommandManager only pays attention to certain conditions in determining when the command target has changed, such as change in keyboard focus.

As you can see this is very vage and there are situations, where the CommandManager simply cannot know:

In situations where the CommandManager does not sufficiently determine a change in conditions that cause a command to not be able to execute, InvalidateRequerySuggested can be called to force the CommandManager to raise the RequerySuggested event.

In summary, yes, the RequerySuggested event is raised when user input changes and on other input related events e.g. in a TextBox or bound properties change, but not in all situations. From a different perspective, the CommandManager determines when it needs to raise the event only on general triggers, so often it will invalidate all can execute states, although most or even no command might even be affected. It is not targeted to the specific case, like a distinct property that you want to observe for changes, but like a watering pot. This can of course make a difference in performance, although negligible in the majority of applications.

I put breakpoint on CanExecute methods of my commands and saw that they work forever in debug

Yes and by now you know exactly why. In debug mode when a breakpoint is hit, the debugger or IDE is brought into foreground meaning the keyboard focus changes. When you switch back to your application being debugged, the keyboard focus changes again...and again...and again. Since the CommandManager raises the RequerySuggested event on keyboard focus, you will constantly trigger CanExecute and hit the breakpoint. The same may happen on window activation as well.

A Different Command Approach

There is another very common approach for notifying can execute changed. In your example, you rely on the CommandManager to do its best for all commands. However, you could also take responsibility yourself and explicitly invalidate can execute through another public method.

public class RelayCommand<T> : ICommand
{
   private readonly Predicate<T> _canExecute;
   private readonly Action<T> _execute;

   public RelayCommand(Action<T> execute) : this(execute, null)
   {
      _execute = execute;
   }

   public RelayCommand(Action<T> execute, Predicate<T> canExecute)
   {
      _execute = execute ?? throw new ArgumentNullException(nameof(execute));
      _canExecute = canExecute;
   }

   public bool CanExecute(object parameter)
   {
      return _canExecute == null || _canExecute((T)parameter);
   }

   public void Execute(object parameter)
   {
      _execute((T)parameter);
   }

   public event EventHandler CanExecuteChanged;

   public void RaiseCanExecuteChanged()
   {
      CanExecuteChanged?.Invoke(this, EventArgs.Empty);
   }
}

Here you assume that you know exactly when to update your own command. Instead of asking "Dear command manager, please tell me when to update" you say "Update these specific commands now". For this you have to call RaiseCanExecuteChanged for each affected command in your properties.

public string UserInput
{
   get { return _userInput; }
   set
   {
      if (UserInput != value)
      {
         _userInput = value;
         OnPropertyChanged("UserInput");
         AddTextCmnd.RaiseCanExecuteChanged();
         ClearTextCmnd.RaiseCanExecuteChanged();
         ShowResultCmnd.RaiseCanExecuteChanged();
      }
   }
}

This mechanism has a few advantages over the RequerySuggested event.

You decide exactly when commands are updated.
Only commands that are really affected are updated.
It is much easier to comprehend the dependencies in your view model.
Potential performance benefit from reducing unnecessary updates.
No reliance on an external component and hidden magic or vage assumptions.

A Hint On DRY and Reusability

As of now, all of your commands contain duplicated logic, violating the Don't repeat yourself principle, in short DRY. This makes maintenance much harder. Instead you should at least extract a common, reusable command type as shown above to improve your code. You do not even have to implement one yourself, there are plenty of frameworks, libraries and NuGet packages available already, e.g. Microsoft.Toolkit.Mvvm, see its documentation here.

Here is an example how it would look like for your clear text command:

public ViewModel()
{
   ClearTextCmnd = new RelayCommand(ExecuteClearText, CanExecuteClearText);
   // ...other commands.
}

public ICommand ClearTextCmnd { get; set; }

public bool CanExecuteClearText()
{
   return UserInput != "0";
}

public void ExecuteClearText()
{
   UserInput = "0";
}

For simplicity I used a RelayCommand implementation without an extra parameter, as you do not use it currently. It is the same as the command above, only without the parameter for each method.