Is it poor form for a C# class to subscribe to its own published events?

Question

I'm probably just being neurotic, but I regularly find myself in situations in which I have class that publishes an event, and I find it convenient to subscribe to this event from within the class itself (e.g. in the constructor), rather than only subscribing from external classes.

This sounds reasonable to me, but I can't help the nagging feeling that it's a poor practice, for the simple reason that I'm always faced with the question: "Why not perform the actions that you'd provide in the event handler in the code which fires the event?"

public class Button
{
   public Button()
   {
      this.Click += someHandler; // bad practice?
   }

   public event EventHandler Click;

   public void HandleInput()
   {
      if (someInputCondition)
      {
         // Perform necessary actions here rather than 
         // subscribing in the constructor?
         this.Click(this, ...);
      }
   }
}

Are there any drawbacks to subscribing to your own events?

Answer 1

I see no problem with this. But if you handle the events in the same class you could also override the event method:

protected override void OnClick(Eventargs e)
{
   base.OnClick(e);
}

This is more efficient and gives you the power to swallow the event if necessary (simply not calling base.OnClick()).

Answer 2

The issue is that “someHandler” will change the state of your object. Do you want this state changing before or after any “external” code is run by the event?

It is not clear at what point the state change will be make if you subscribe to the event, however calling it in “HandleInput()” make it a lot clearer when it will be called.

(Also it is more normal to call “HandleInput()”, “OnClick” and make it virtual so sub classes can override it)

After saying the above, normally there is no great harm in subscribing to your own event; in UI classes that represent forms it is very common, otherwise it tend to “surprise” a lot of people that read your code.

Answer 3

There's a very exotic problem due to internal optimization when doing this. Due to the optimization adding/removing event handlers is not thread safe. It only applies to events that are used by the declaring type like in your example.

Fortunately this has been changed with 4.0, but if you're on previous version, you could experience this.

Answer 4

if you take the ordinary System.Windows.Form class as an example,
when you want to handle the Form_Load event (using visual studio designer), it is handled in the class of the Form itself !

this.Load += new System.EventHandler(this.Form1_Load);

 private void Form1_Load(object sender, EventArgs e)
 {
 }

so i think it is not a problem at all !!.

Answer 5

If your button class should be the first which receives the click event, you should write your code in the event method, eg.:

protected virtual void OnClick(EventArgs e)
{
    //insert your code here

    if(this.Click != null)
    {
        this.Click(this, e);
    }
}

but if it's not necessary that your class is the first reciever, you can subscribe to the event normally.

Answer 6

This sounds reasonable to me, but I can't help the nagging feeling that it's a poor practice, for the simple reason that I'm always faced with the question: "Why not perform the actions that you'd provide in the event handler in the code which fires the event?"

To answer that question, consider partial class scenarios. Suppose you have a base type B. You run an automated tool that decorates B by extending it to derived class D. Your tool generates a partial class so that developers consuming D can further customize it for their own purposes.

In that case, it seems perfectly reasonable that the user-authored side of D would want to sign up to be called when events declared by B or the machine-generated side of D are raised by the machine-generated side of D.

That was the scenario we found ourselves in when designing VSTO many years ago. As it turns out, it was not difficult to do this in C# but it was quite tricky to get it all working in VB. I believe VB has made some tweaks to their event subscription model to make this easier.

That said: if you can avoid this, I would. If you're just making an event for internal subscription that seems like a bad code smell. Partial methods in C# 3 help out greatly here, since they make it easy and low-cost for the machine-generated side to call little notification functions in the user-generated side, without having to go to the trouble of publishing an event.