Est-ce que l'implémentation de Josh Smith du RelayCommand est défectueuse?

Question

Considérez la référence de l'article de Josh Smith sur les applications WPF avec le modèle de conception Modèle-Vue-ModèleVue, en particulier l'implémentation d'exemple d'un RelayCommand (dans la Figure 3). (Pas besoin de lire l'intégralité de l'article pour cette question.)

En général, je pense que l'implémentation est excellente, mais j'ai une question sur la délégation des abonnements de CanExecuteChanged à l'événement RequerySuggested du CommandManager. La documentation sur RequerySuggested indique :

Comme cet événement est statique, il ne conservera le gestionnaire que sous forme de référence faible. Les objets qui écoutent cet événement doivent conserver une référence forte à leur gestionnaire d'événement pour éviter qu'il ne soit ramassé par le ramasse-miettes. Cela peut être réalisé en ayant un champ privé et en assignant la variable du gestionnaire en valeur avant ou après avoir attaché cet événement.

Pourtant, l'implémentation d'exemple de RelayCommand ne maintient pas une telle référence au gestionnaire abonné :

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

1. Cela fuit-il la référence faible jusqu'au client de RelayCommand, nécessitant que l'utilisateur de RelayCommand comprenne l'implémentation de CanExecuteChanged et maintienne lui-même une référence vivante ?

2. Si tel est le cas, est-ce logique, par exemple, de modifier l'implémentation de RelayCommand pour être quelque chose comme ce qui suit pour atténuer le risque de collecte prématurée des déchets du souscripteur de CanExecuteChanged :

   // Cet événement ne se produit jamais réellement. C'est purement pour la gestion du cycle de vie.
   private event EventHandler canExecChangedRef;
   public event EventHandler CanExecuteChanged
   {
       add 
       { 
           CommandManager.RequerySuggested += value;
           this.canExecChangedRef += value;
       }
       remove 
       {
           this.canExecChangedRef -= value;
           CommandManager.RequerySuggested -= value; 
       }
   }

Answer 1

J'ai trouvé la réponse dans le commentaire de Josh sur son article "Comprendre les commandes routées" :

[...] vous devez utiliser le modèle WeakEvent dans votre événement CanExecuteChanged. Cela est nécessaire car les éléments visuels vont s'accrocher à cet événement, et étant donné que l'objet commande pourrait ne jamais être collecté par le garbage collector tant que l'application ne se ferme pas, il existe un réel risque de fuite mémoire. [...]

L'argument semble être que les implémenteurs de CanExecuteChanged doivent uniquement détenir faiblement les gestionnaires enregistrés, puisque les Visuals de WPF sont trop bêtes pour se désaccrocher eux-mêmes. Cela est le plus facilement mis en œuvre en déléguant au CommandManager, qui le fait déjà. Probablement pour la même raison.

Answer 2

Je pense aussi que cette implémentation est défectueuse, car elle fuite définitivement la référence faible à l'événement. C'est en réalité très mauvais.
J'utilise la MVVM Light toolkit et la RelayCommand implémentée dedans et elle est implémentée exactement comme dans l'article.
Le code suivant n'invoquera jamais 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;
    }
}

Cependant, si je le change comme ceci, cela fonctionnera :

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;
    }
}

La seule différence ? Tout comme indiqué dans la documentation de CommandManager.RequerySuggested, je sauvegarde l'événement dans un champ.

Answer 3

Eh bien, d'après Reflector, cela est implémenté de la même manière dans la classe RoutedCommand, donc je suppose que tout est en ordre... à moins que quelqu'un de l'équipe WPF n'ait commis une erreur ;)

Answer 4

Je crois que c'est faux.

En redirigeant les événements vers CommandManager, vous obtenez le comportement suivant

Cela garantit que l'infrastructure de commande WPF demande à tous les objets RelayCommand s'ils peuvent s'exécuter chaque fois qu'elle demande les commandes intégrées.

Cependant, que se passe-t-il lorsque vous souhaitez informer tous les contrôles liés à une seule commande de réévaluer l'état CanExecute? Dans cette implémentation, vous devez aller vers CommandManager, ce qui signifie

Chaque liaison de commande dans votre application est réévaluée

Cela inclut toutes celles qui n'ont pas d'importance, celles pour lesquelles l'évaluation de CanExecute a des effets secondaires (comme l'accès à la base de données ou les tâches longues), celles qui attendent d'être collectées... C'est comme utiliser un marteau-piqueur pour enfoncer un clou.

Vous devez sérieusement prendre en compte les conséquences de le faire.

Answer 5

Je pourrais passer à côté du point ici, mais ne constitue-t-il pas la référence forte à l'event handler dans le constructeur suivant ?

    _canExecute = canExecute;