Prism: `DelegateCommand.ObservesCanExecute()` fails to raise `CanExecuteChanged`

Created on 23 Jun 2016  路  7Comments  路  Source: PrismLibrary/Prism

It appears that ObservesCanExecute() on DelegateCommand only works if you use a member of the instantiating class.

Repro

Consider the following ViewModel:

``` C#
public class ViewModel: INotifyPropertyChanged
{
public event PropertyChangedEventHandler PropertyChanged;

    public ICommand Command { get; set; }

    private bool _enabler;
    public bool Enabler
    {
        get { return _enabler; }
        set
        {
            if (value == _enabler)
            {
                return;
            }
            _enabler = value;
            PropertyChanged.Raise(this, () => Enabler);
        }
    }
}
And this test:

``` C#
[Test]
        public void ShouldEnableCommand()
        {
            var raiseCounter = 0;
            var vm = new ViewModel();
            vm.Command = new DelegateCommand(() => { }).ObservesCanExecute(o => vm.Enabler);
            vm.Command.CanExecuteChanged += (_, __) => raiseCounter++;

            vm.Command.CanExecute(null).Should().BeFalse();
            raiseCounter.Should().Be(0);

            vm.Enabler = true;
            vm.Command.CanExecute(null).Should().BeTrue();
            raiseCounter.Should().Be(1);
        }

This test fails!

However if I instantiate Command in the ViewModel ctor it passes, as:

C# public ViewModel() { Command = new Prism.Commands.DelegateCommand(() => { }).ObservesCanExecute(o => Enabler); }

:anguished: :open_mouth: :confounded:

Package info

  • Platform: .net 4.5.2
  • Prism version: 6.1.0
  • NUnit: 3.2.1
  • FluentAssertions: 4.5.0

Most helpful comment

Just an update. Support for nested properties has been added in the latest code base. Should be available in the next major release

All 7 comments

Correct. This is the expected behavior. The Observes functionality hooks into the INPC of the defining class, so in this case it would be your test class. The feature also does not support complex objects like you have in your test. ObServesProperty(() => Class.PropertyName); would not work even if you defined the command within the VM, because it does not hook into the INPC of the property being observed.

I am sure there's a good reason, but why not hook to the INPC of the property's containing type?

That would require you to provide the defining type of the property to the function which would make the API more complicated for the 95% use case, and honestly it isn't meant to be used the way you are trying to use it.

@brianlagunas a method that takes Func<object, bool> and is called ObservesCanExecute and does not say anything about these points in it's doc can be interpreted in any way by the user.
In my case I split my INPC ViewModel types into two separate types- one containing state (ViewModels) and the other behaviours (ViewControllers), the former is instantiated in the latter. Therefore I will always be instantiating a DelegateCommand in a ViewController type and I would want the ObserveCanExecute on a property in the ViewModel type. Would it be useful to enhance the API to provide a defining type?

No, because that is asking for memory leaks. You will be hooking into the INPC event of an object, and that will root that object unless you explicitly unhook the event.

Just an update. Support for nested properties has been added in the latest code base. Should be available in the next major release

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Was this page helpful?
0 / 5 - 0 ratings