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

Question

Nous sommes tous familiers avec l'horreur qui est en C# d'événements de la déclaration. Pour assurer le thread de sécurité, la norme est d'écrire quelque chose comme ceci:

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

Récemment dans certains autres question sur ce forum (dont je ne peux pas trouver), quelqu'un a fait remarquer que les méthodes d'extension peut être utilisé 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 de 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? Sommes-nous oublié quelque chose en n'ayant pas la norme Sur la méthode? (Une chose que je constate, c'est qu'il ne fonctionne pas avec les événements qui ont explicites ajouter/supprimer le code.)

Answer 1

Il faudra encore travailler avec les événements qui ont explicitement ajouter/supprimer - vous avez juste besoin d'utiliser le délégué de la variable (ou cependant vous avez stocké le délégué) au lieu du nom de l'événement.

Cependant, il ya un moyen plus facile de faire thread-safe - initialiser avec un no-op gestionnaire:

public event EventHandler SomethingHappened = delegate {};

L'impact sur les performances de l'appel d'une supplémentaire délégué sera négligeable, et il vous rend le code plus facile.

Par ailleurs, dans votre méthode d'extension vous n'avez pas besoin d'un supplément variable locales, vous pourriez 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 comme un nom de paramètre, mais il ne change pas vraiment de la partie appelante à tous, donc faire ce que vous voulez :)

EDIT: Comme pour le "OnXXX" méthode: envisagez-vous de vos classes dérivées à partir? De mon point de vue, la plupart des classes doivent être scellés. Si vous n', voulez-vous ces classes dérivées pour être en mesure de déclencher l'événement? Si la réponse à une de ces questions est "non", alors ne vous embêtez pas. Si la réponse aux deux questions est "oui", puis faire :)

Answer 2

[Voici une pensée]

Il suffit d’écrire le code une fois de la manière recommandée et d’en finir. Dans ce cas, vous ne laisserez pas vos collègues en train de regarder le code en pensant que vous avez fait quelque chose de mal.

[J'ai lu plus de messages essayant de trouver des moyens d'écrire un gestionnaire d'événements que je n'ai jamais dépensé pour écrire un gestionnaire d'événements.]

Answer 3

Moins de code, plus lisible. Moi comme.

Si les performances ne vous intéressent pas, vous pouvez déclarer votre événement comme suit pour éviter le contrôle nul:

 public event EventHandler SomethingHappened = delegate{};
 

Answer 4

Vous n'êtes pas "assurer" la sécurité des threads en attribuant le gestionnaire à une variable locale. Votre méthode pourrait encore être interrompue après la cession. Si, par exemple, la classe qui utilisé pour écouter l'événement obtient cédés au cours de l'interruption, vous êtes à l'appel d'une méthode dans un éliminées de la classe.

Vous êtes en train d'enregistrer vous-même à partir d'une référence nulle exception, mais il ya des façons plus faciles de le faire, comme Jon Skeet et cristianlibardo ont souligné dans leurs réponses.

Une autre chose est que pour les non-scellé classes, la OnFoo méthode doit être virtuel qui je ne pense pas possible avec les méthodes d'extension.

Answer 5

La chose drôle au sujet de ce à mon humble avis est que, après la suite de Jon Skeet la simplification de votre proposition, la méthode d'extension ressemble à ceci:

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

Si vous avez l'intention de remplacer une ligne de code, à savoir

SomethingHappened(this, new EventArgs(...));

par une autre ligne de code, à savoir:

SomethingHappend.RaiseEvent(this, new EventArgs(...));

qui ajoute ".RaiseEvent".Caractères de longueur, plus l'extension de la méthode de définition, à l'aide d'instruction et, éventuellement, une référence à l'assembly contenant la méthode d'extension.

Suis-je tort?