DelegateCommand.CanExecute method not behaving correctly

Question

I'm binding a control to a DelegateCommand and the CanExecute portion of it is not working properly. I am using the Prism libraries. Can anyone tell me why?

Command declaration and instantiation:

public PlayerManagementViewModel(DatabaseManager dbManager)
{
    _dbManager = dbManager;
    this.ResetUpToDateStatusCommand = new DelegateCommand(() => this.ResetXpUpToDateStatus());
    this.DeletePlayerCommand = new DelegateCommand(() => this.DeleteSelectedPlayer(), () => SelectedPlayer != null);
    this.RefreshPlayers();
}

public ICommand DeletePlayerCommand { get; private set; }

SelectedPlayer definition:

public Player SelectedPlayer
{
    get { return _selectedPlayer; }
    set
    {
        SetProperty(ref this._selectedPlayer, value);
        this.OnPropertyChanged(() => this.FormattedPlayerStatus);
    }
}

The weird thing is that if you look at the line above the DeletePlayerCommand instantiation, that line works just fine. I don't get any CanExecute behavior out of it, but at least it works. As is, the DeletePlayerCommand command never fires off, even with a breakpoint, unless I remove the CanExecute portion of the constructor entirely.

Can anyone please explain to me why this is or what I'm doing wrong?

I have not used prism much but is the fact that `OnPropertyChanged` is pointing at `FormattedPlayerStatus` instead of something related to SelectedPlayer correct? — Scott Chamberlain, Jan 29 '15 at 22:51
I'm not familiar with this particular `DelegateCommand` helper class, but basic `ICommand` requires you to actively indicate when users should re-check `CanExecute`, by raising the `CanExecuteChanged` event. If that event is never raised, and the command was disabled before, it is assumed to still be disabled. Does `DelegateCommand` have some means of raising that event? — , Jan 29 '15 at 22:53
Scott: `SetProperty` also raises an `OnPropertyChanged`. The second line is to raise a second one specifically for a read-only property and shouldn't affect anything here. — Ari Roth, Jan 29 '15 at 22:58
hvd: That's what I'd think too, as the functionality you describe is there. However, with the two-argument constructor, *nothing* is raised. The control remains enabled, and clicking on it fails to hit a breakpoint in the appropriate locations. — Ari Roth, Jan 29 '15 at 22:59
Perhaps the binding to that command has a typo. Did you check your output window for binding errors? — BradleyDotNET, Jan 29 '15 at 23:05
http://www.projky.com/entlib/6.0/Microsoft/Practices/EnterpriseLibrary/Configuration/Design/ViewModel/Commands/DelegateCommand.cs.html You can see the RaiseCanExecuteChanged method — eran otzap, Jan 29 '15 at 23:32

Sergey Vyacheslavovich Brunov · Answer 1 · 2015-01-29T23:59:14.457

5

If the CanExecute function of the DeletePlayerCommand is () => SelectedPlayer != null, then there must be a DelegateCommandBase.RaiseCanExecuteChanged Method call when SelectedProperty value is changed:

Raises CanExecuteChanged on the UI thread so every command invoker can requery to check if the command can execute.

The appropriate UI-element (with data-bound command) is a command invoker.

To summarize, the implementation of the SelectedPlayer property should be updated as follows:

class PlayerManagementViewModel : BindableBase
{
    private Player _selectedPlayer;
    private readonly DelegateCommand _deletePlayerCommand;

    public PlayerManagementViewModel(...)
    {
        _deletePlayerCommand = new DelegateCommand(() => DeleteSelectedPlayer(), () => SelectedPlayer != null);
    }

    public ICommand DeletePlayerCommand
    {
        get { return _deletePlayerCommand; }
    }

    public Player SelectedPlayer
    {
        get { return _selectedPlayer; }
        set
        {
            SetProperty(ref _selectedPlayer, value);
            OnPropertyChanged(() => FormattedPlayerStatus);
            _deletePlayerCommand.RaiseCanExecuteChanged();
        }
    }
}

edited Jan 29 '15 at 23:59

answered Jan 29 '15 at 23:30

Sergey Vyacheslavovich Brunov

17,291
7
48
81

3

It may be worth noting, the `DeletePlayerCommand` would need to be of type `DelegateCommand` and not `ICommand` for this to work. Either that or use a backing field of type `DelegateCommand` and expose the `ICommand` returning this backing field. – Lukazoid Jan 29 '15 at 23:34
@Sergey: Is the second `OnPropertyChanged` really necessary in `SelectedPlayer`? My understanding was that `SetProperty` would raise it automatically when called. – Ari Roth Jan 29 '15 at 23:45
@AriRoth, good question. First of all, could you please post the implementation of `SetProperty(...)` method or give a reference to the documentation? – Sergey Vyacheslavovich Brunov Jan 29 '15 at 23:49
@AriRoth, right. Thank you! The `OnPropertyChanged()` method call for `SelectedPlayer` is redundant. Updated. – Sergey Vyacheslavovich Brunov Jan 30 '15 at 00:00
I'll give it a shot this weekend and see if this helps. I'm not using backing fields right now, and the property itself is an ICommand rather than a DelegateCommand, and I'm not raising events explicitly, so we'll see. I'll keep you all posted here. – Ari Roth Jan 30 '15 at 19:34
@Sergey: No dice. Adding the second argument to the DelegateCommand constructor makes the entire command fail to work. It doesn't matter if I'm using an Automatic or backing field for a property, or if the property is an ICommand or DelegateCommand. – Ari Roth Jan 30 '15 at 23:06
@AriRoth, what do yo mean by "fail to work"? Please provide more details. – Sergey Vyacheslavovich Brunov Jan 31 '15 at 14:51
@Sergey See my original post. The command fails to bind. It doesn't execute nor does it behave according to its own CanExecute method. The binding just does nothing at all. But if I remove the CanExecute part the binding works just fine without touching the XAML, so I know it's not the XAML part of the binding. – Ari Roth Jan 31 '15 at 18:57
@AriRoth, thank you for the information. What about the value of `SelectedPlayer` property? Is the property data-bound and has not null reference as a value? Also, please try to test it using `ViewModel` constructor: set the value of `SelectedPlayer` property as, for example, a new instance of `Player` class. – Sergey Vyacheslavovich Brunov Feb 01 '15 at 02:33

score 0 · Answer 2 · edited May 23 '17 at 12:08

0

It is the way PRISM DelegateCommnd is designed. refer CanExecuteChanged event of ICommand.

Alternatively you can derive the DelegateCommand to overcome the limitation. refer the below code.

 class DelegateCmdEx : DelegateCommand
{       
    public DelegateCmdEx(Action executeMethod):base(executeMethod)
    {

    }

    public DelegateCmdEx(Action executeMethod, Func<bool> canExecuteMethod)
        : base(executeMethod, canExecuteMethod)
    {

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

edited May 23 '17 at 12:08

Community

1
1

answered Jan 29 '15 at 23:29

Ayyappan Subramanian

5,348
1
22
44

1

I wouldn't say this was a problem with Prism's `DelegateCommand`, it just hasn't been designed to automatically raise `CanExecuteChanged`. – Lukazoid Jan 29 '15 at 23:38
1

Assuming you have hundreds of commands, doing so will lead to performance drops, because the command manager will re-estimate the availability of **all** commands on single command availability change. – dymanoid Jan 30 '15 at 08:29
@Lukazoid: Agreed Edited my answer. – Ayyappan Subramanian Jan 30 '15 at 17:24
@dymanoid: Agreed. But in case of less number of commands i think this method will be handy. – Ayyappan Subramanian Jan 30 '15 at 17:25

DelegateCommand.CanExecute method not behaving correctly

2 Answers2