Élever des événements C# avec une méthode d'extension - est-ce mauvais?

Question

Nous connaissons tous l'horreur de la déclaration d'événement C#. Pour assurer la sécurité des threads, the standard is to write something like this:

public event EventHandler SomethingHappened; 
protected virtual void OnSomethingHappened(EventArgs e) 
{    
    var handler = SomethingHappened; 
    if (handler != null) 
     handler(this, e); 
} 

Récemment une autre question sur ce forum (que je ne peux pas trouver maintenant), quelqu'un a fait remarquer que les méthodes prolongation pourraient être utilisés bien dans ce scénario. Voici une façon de le faire:

static public class EventExtensions 
{ 
    static public void RaiseEvent(this EventHandler @event, object sender, EventArgs e) 
    { 
     var handler = @event; 
     if (handler != null) 
      handler(sender, e); 
    } 
    static public void RaiseEvent<T>(this EventHandler<T> @event, object sender, T e) 
     where T : EventArgs 
    { 
     var handler = @event; 
     if (handler != null) 
      handler(sender, e); 
    } 
} 

Avec ces méthodes d'extension en place, tout ce que vous devez déclarer et déclencher un événement est quelque chose comme ceci:

public event EventHandler SomethingHappened; 

void SomeMethod() 
{ 
    this.SomethingHappened.RaiseEvent(this, EventArgs.Empty); 
} 

Ma question: Est-ce une bonne idée ? Manquons-nous quelque chose en n'ayant pas la méthode On standard? (Une chose que je remarque est que cela ne fonctionne pas avec les événements qui ont explicitement ajouter/supprimer du code.)

Answer 1

Cela fonctionne toujours avec les événements qui ont un ajout/suppression explicite - vous avez juste besoin d'utiliser la variable de délégué (ou bien vous avez enregistré le délégué) au lieu du nom de l'événement.

Cependant, il y a un moyen plus facile de le rendre thread-safe - initialiser avec un gestionnaire no-op:

public event EventHandler SomethingHappened = delegate {}; 

Le succès de la performance d'appeler un délégué supplémentaire sera négligeable, et il fait que le code plus facile.

Par ailleurs, dans votre méthode d'extension vous n'avez pas besoin d'une variable locale supplémentaire - vous pouvez simplement faire:

static public void RaiseEvent(this EventHandler @event, object sender, EventArgs e) 
{ 
    if (@event != null) 
     @event(sender, e); 
} 

static public void RaiseEvent<T>(this EventHandler<T> @event, object sender, T e) 
    where T : EventArgs 
{ 
    if (@event != null) 
     @event(sender, e); 
} 

Personnellement, je ne voudrais pas utiliser un mot-clé comme nom de paramètre, mais il doesn Ne changez pas du tout le côté appelant, alors faites ce que vous voulez :)

EDIT: Comme pour la méthode "OnXXX": avez-vous prévu que vos classes soient dérivées? À mon avis, la plupart des classes devraient être scellées. Si vous faites, voulez-vous que ces classes dérivées soient en mesure d'élever l'événement? Si la réponse à l'une ou l'autre de ces questions est «non», ne vous embêtez pas. Si la réponse aux deux est "oui" alors faites :)

Answer 2

Moins de code, plus lisible. Moi comme.

Si vous n'êtes pas intéressé par les performances que vous pouvez déclarer votre événement comme celui-ci pour éviter la vérification null:

public event EventHandler SomethingHappened = delegate{}; 

Answer 3

Vous n'êtes pas « assurer » sécurité des threads en attribuant le gestionnaire à une locale variable. Votre méthode pourrait encore être interrompue après l'affectation. Si, par exemple, la classe qui a écouté l'événement est éliminée pendant l'interruption, vous appelez une méthode dans une classe éliminée. Vous vous enregistrez d'une exception de référence nulle, mais il existe des moyens plus faciles de le faire, comme Jon Skeet et cristianlibardo l'ont souligné dans leurs réponses.

Une autre chose est que pour les classes non scellées, la méthode OnFoo devrait être virtuelle, ce que je ne pense pas être possible avec les méthodes d'extension.

Answer 4

[Voilà une pensée]

Il suffit d'écrire le code une fois de la façon recommandée et être fait avec elle. Alors, vous ne confondrez pas vos collègues qui regardent le code en pensant que vous avez fait quelque chose de mal?

[J'ai lu plus de messages essayant de trouver des façons de contourner écrire un gestionnaire d'événements que je ne passe jamais à écrire un gestionnaire d'événements.]

Answer 5

maintenant C# 6 est ici, il y a un moyen de tirer plus compact, thread-safe un événement:

SomethingHappened?.Invoke(this, e); 

Invoke() est appelée que si les délégués sont inscrits pour l'événement (il est non nul), grâce à l'opérateur null conditionnelle « ? ». Le problème de threading que le code "handler" de la question cherche à résoudre est évité ici car, comme dans ce code, SomethingHappened n'est accédé qu'une seule fois, donc il n'y a aucune possibilité de le rendre nul entre test et invocation .

Cette réponse est peut-être tangente à la question initiale, mais très relevent pour ceux qui recherchent une méthode plus simple pour déclencher des événements.