(WPF) RelayCommand.CanExecuteChanged event does not keep a strong reference

Jul 31, 2012 at 1:34 PM

Hi

in RelayCommand and RelayCommand<T> implementation I've found that you attach event handlers to CommandManager.RequerySuggested event but you don't keep a strong reference, as suggested in MSDN: http://msdn.microsoft.com/en-us/library/system.windows.input.commandmanager.requerysuggested.aspx

In fact (at least in my project) if I call MyCommand.RaiseCanExecuteChanged() on a command used in EventToCommand action it does nothing, and MustToggleIsEnabled is effectively non-functional.

I've changed your implementation with this:

        private EventHandler _requerySuggestedLocal;
        /// <summary>
        /// Occurs when changes occur that affect whether the command should execute.
        /// </summary>
        public event EventHandler CanExecuteChanged
        {
            add
            {
                if (_canExecute != null)
                {
                    // add event handler to local handler backing field in a thread safe manner
                    EventHandler handler2;
                    EventHandler canExecuteChanged = this._requerySuggestedLocal;
                    do
                    {
                        handler2 = canExecuteChanged;
                        EventHandler handler3 = (EventHandler)Delegate.Combine(handler2, value);
                        canExecuteChanged = Interlocked.CompareExchange<EventHandler>(ref this._requerySuggestedLocal, handler3, handler2);
                    } while (canExecuteChanged != handler2);

                    CommandManager.RequerySuggested += value;
                }
            }

            remove
            {
                if (_canExecute != null)
                {
                    // removes an event handler from local backing field in a thread safe manner
                    EventHandler handler2;
                    EventHandler canExecuteChanged = this._requerySuggestedLocal;
                    do
                    {
                        handler2 = canExecuteChanged;
                        EventHandler handler3 = (EventHandler)Delegate.Remove(handler2, value);
                        canExecuteChanged = Interlocked.CompareExchange<EventHandler>(ref this._requerySuggestedLocal, handler3, handler2);
                    } while (canExecuteChanged != handler2);

                    CommandManager.RequerySuggested -= value;
                }
            }
        }

And now my EventToCommand actions are working fine.

Cheers
Paolo

Coordinator
Jul 31, 2012 at 3:07 PM

Hi,

Good feedback, I added that to my backlog. Thanks!

Laurent

Aug 2, 2012 at 9:41 AM

Backlog?

already mentioned this same thing end of 2010, then v3, you promised for v4 (so I had to copy sources and change myself) and still?

Coordinator
Aug 2, 2012 at 10:03 AM

Hi,

I understand your frustration. MVVM Light is a project that I manage during my free time. As I mentioned earlier, deciding which features to implement in which version is not always easy, and I decided to release V4 before I was able to finish all the items on my backlog. Some things take time, especially testing, making sure that it doesn't break anything, etc. Not looking for excuses here, just trying to let you see the reality of a project like MVVM Light. I am also very conservative in accepting contributions (meaning that I generally don't) because I want to avoid spending time managing different contributors, and because I want to keep the code tight and small.

I am always grateful for people reaching out, and due to the feedback regarding this particular issue, I bumped the priority so that it will be handled in the next release, which will come much faster than V3 -> V4. I am aware I took much too long to release V4 and I want to avoid this error in the future.

Thanks,

Laurent

Aug 2, 2012 at 11:47 AM

Hi,

Thanks for your answer.

You told me more or less same thing in 2010 (and I can accept what you say)

But, at that time I have also sent you the solution that I implemented, if I remember well, just adding some lines to your code…. So providing the solution was quite easy and fast, and hey, MVVM Light is a nice framework so we are using it in real applications…

Sincerely yours,

Geert

Mobile: +32(486)434324

Skype: geertdeprez

Please think about the environment before printing this e-mail.

Coordinator
Aug 2, 2012 at 12:43 PM

I know how you feel Geert and I agree with you, I should have done this earlier. I am going in vacation tomorrow, and I will take the occasion to push a new version with that change inside. 

Cheers,

Laurent