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

Question

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

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

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

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

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

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

2. Если это так, имеет ли смысл, например, изменить реализацию RelayCommand на что-то вроде следующего, чтобы уменьшить потенциальный преждевременный GC абонента 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

Я нашел ответ в комментарии Джоша к его статье " Understanding Routed Commands ":

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

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

Answer 2

Я тоже считаю, что эта реализация имеет недостатки , потому что она определенно пропускает слабую ссылку на обработчик событий. Это на самом деле очень плохо.
Я использую инструментарий MVVM Light и реализованный в нем 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 3

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

Answer 4

Я считаю, что это неправильно.

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

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

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

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

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

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

Answer 5

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

    _canExecute = canExecute;