Является ли реализация Джошем Смитом RelayCommand ошибочной?

Question

Рассмотрим ссылку Статья Джоша Смита "Приложения WPF с шаблоном проектирования Model-View-ViewModel", в частности , пример реализации RelayCommand (На рисунке 3).(Для ответа на этот вопрос нет необходимости читать всю статью целиком.)

В целом, я думаю, что реализация отличная, но у меня есть вопрос по поводу делегирования CanExecuteChanged подписки на CommandManager's RequerySuggested событие.Тот самый документация для RequerySuggested состояния:

Поскольку это событие статично, оно будет храниться только в обработчике как слабая ссылка.Объекты, которые прослушивают это событие, должны сохранять надежную ссылку на свой обработчик события, чтобы избежать сбора мусора.Это может быть достигнуто путем создания поля private и назначения обработчика в качестве значения до или после присоединения к этому событию.

Тем не менее, примерная реализация RelayCommand не поддерживает ничего подобного для подписанного обработчика:

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

1. Приводит ли это к утечке слабой ссылки вплоть до RelayCommandклиент, требующий, чтобы пользователь RelayCommand понимать реализацию CanExecuteChanged и сами поддерживаете живую ссылку?

2. Если да, имеет ли смысл, например, изменять реализацию RelayCommand быть чем-то вроде следующего, чтобы смягчить потенциальную преждевременную гибель CanExecuteChanged подписчик:

   // This event never actually fires.  It's purely lifetime mgm't.
   private event EventHandler canExecChangedRef;
   public event EventHandler CanExecuteChanged
   {
       add 
       { 
           CommandManager.RequerySuggested += value;
           this.canExecChangedRef += value;
       }
       remove 
       {
           this.canExecChangedRef -= value;
           CommandManager.RequerySuggested -= value; 
       }
   }
   

Answer 1

Я тоже считаю, что эта реализация является ошибочной, потому что это определенно приводит к утечке слабой ссылки на обработчик событий.Это что-то на самом деле очень плохое.
Я использую MVVM Light toolkit и RelayCommand реализовано в нем так же, как и в статье.
Следующий код никогда не будет вызван OnCanExecuteEditChanged:

private static void OnCommandEditChanged(DependencyObject d, 
                                         DependencyPropertyChangedEventArgs e)
{
    var @this = d as MyViewBase;
    if (@this == null)
    {
        return;
    }

    var oldCommand = e.OldValue as ICommand;
    if (oldCommand != null)
    {
        oldCommand.CanExecuteChanged -= @this.OnCanExecuteEditChanged;
    }
    var newCommand = e.NewValue as ICommand;
    if (newCommand != null)
    {
        newCommand.CanExecuteChanged += @this.OnCanExecuteEditChanged;
    }
}

Однако, если я изменю его таким образом, это сработает:

private static EventHandler _eventHandler;

private static void OnCommandEditChanged(DependencyObject d,
                                         DependencyPropertyChangedEventArgs e)
{
    var @this = d as MyViewBase;
    if (@this == null)
    {
        return;
    }
    if (_eventHandler == null)
        _eventHandler = new EventHandler(@this.OnCanExecuteEditChanged);

    var oldCommand = e.OldValue as ICommand;
    if (oldCommand != null)
    {
        oldCommand.CanExecuteChanged -= _eventHandler;
    }
    var newCommand = e.NewValue as ICommand;
    if (newCommand != null)
    {
        newCommand.CanExecuteChanged += _eventHandler;
    }
}

Единственная разница?Точно так же, как указано в документации CommandManager.RequerySuggested Я сохраняю обработчик события в поле.

Answer 2

Я нашла ответ в книге Джоша. Комментарий на его "Понимание Маршрутизируемых команд" статья:

[...] вы должны использовать шаблон WeakEvent в вашем событии CanExecuteChanged .Это связано с тем, что visual elements перехватит это событие, и поскольку объект command может никогда не быть собран для мусора, пока приложение не завершит работу, существует очень реальная вероятность утечки памяти.[...]

Аргумент, по - видимому, заключается в том, что CanExecuteChanged разработчики должны лишь слабо поддерживать зарегистрированные обработчики, поскольку WPF Visuals слишком глупы, чтобы отцепляться друг от друга.Это проще всего реализовать, делегировав CommandManager, который уже делает это.Предположительно, по той же причине.

Answer 3

Ну, согласно Reflector, это реализовано таким же образом в RoutedCommand класс, так что, я думаю, все должно быть в порядке...если только кто-то из команды WPF не допустил ошибку ;)

Answer 4

Я считаю, что это порочно.

Перенаправляя события в CommandManager, вы получаете следующее поведение

Это гарантирует, что командующий WPF инфраструктура запрашивает все объекты RelayCommand , могут ли они выполняться всякий раз, когда он запрашивает встроенные команды.

Однако что происходит, когда вы хотите сообщить всем элементам управления, привязанным к одной команде, о повторной оценке состояния CanExecute?В его реализации вы должны перейти к CommandManager, что означает

Каждая отдельная привязка команды в вашем приложении пересматривается

Это включает в себя все те, которые не имеют ни малейшего значения, те, где оценка CanExecute имеет побочные эффекты (такие как доступ к базе данных или длительно выполняющиеся задачи), те, которые ожидают сбора данных...Это все равно что кувалдой вбивать чертов гвоздь.

Вы должны серьезно обдумать последствия этого.

Answer 5

Возможно, я здесь упускаю суть, но разве следующее не является строгой ссылкой на обработчик событий в конструкторе?

    _canExecute = canExecute;