C# - set empty delegate for event

Question

public event EventHandler MyButtonClick = delegate { };

The construction above allows to not check if there is any subscriber:

public virtual void OnMyButtonClick(EventHandler e)
        {
            this.MyButtonClick(this, e);
        }

in stead of

  public virtual void OnMyButtonClick(EventHandler e)
            { 
                if (MyButtonClick!=null)
                   this.MyButtonClick(this, e);
            }

But is it really a good idea? Is this the only benefit: to not check if any subscriber exists?

UPDATE: Here is example

namespace ConsoleApplication2
{
    public class TestClass
    {
        public event EventHandler MyButtonClick;
            //= delegate { };

        public void OnButtonClick(EventArgs e)
        {
            MyButtonClick(this, e);
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            var testClass = new TestClass();
            //it throws an exception
            testClass.OnButtonClick(new EventArgs());

            // if you add an handler it will call it

            testClass.MyButtonClick += myCustomHandler;
            testClass.OnButtonClick(new EventArgs()); // myCustomHandler has been invoiked

        }

        private static void myCustomHandler(object sender, EventArgs e)
        {
            Console.WriteLine("myCustomHandler has been invoiked");
        }
    }
}

Answer 1

Well, the code you've given here:

public virtual void OnMyButtonClick(EventHandler e)
{ 
    if (MyButtonClick!=null)
       this.MyButtonClick(this, e);
}

isn't thread-safe. If the final subscription is removed after the nullity check but before the invocation, you could end up with a NullReferenceException (depending on whether the "raising" thread sees the change).

So you can change it to this instead:

public virtual void OnMyButtonClick(EventArgs e)
{ 
    var handler = MyButtonClick;
    if (handler != null)
    {
        handler(this, e);
    }
}

... but of course you might forget to do that, and even if you don't, it's cumbersome to do that all over the place, IMO. So yes, while the benefit is "only" to avoid the nullity check, I'd say that's not a bad trade-off in many cases. Anything that makes it harder to make mistakes is a good idea, IMO.

Another alternative is to have an extension method:

public static void SafeInvoke(this EventHandler handler, object sender,
                              EventArgs e)
{
    if (handler != null)
    {
        handler(sender, e);
    }       
}

Then change your calling code to:

public virtual void OnMyButtonClick(EventArgs e)
{
    MyButtonClick.SafeInvoke(this, e);
}

(and use the same code for other events). You'd probably want a generic form for EventHandler<T> as well.

Answer 2

you don't need to do that. If the client that uses you class won't add an handler (subscriber) for MyButtonClick event the code won't throw an exception.

That is how events works (and delegates as there are the same thing) otherwise you would be forced to add an handler to all the events of a class (assuming there are any)

so you can do the below:

public virtual void OnMyButtonClick(EventArgs e)
{ 
   MyButtonClick(this, e);
}

have a look at the example below:

public class TestClass
{
    public event EventHandler MyButtonClick = delegate { };

    public void ButtonClick(EventArgs e)
    {
        MyButtonClick(this,e);
    }
}

class Program
{
    static void Main(string[] args)
    {
        var testClass=new TestClass();
        testClass.ButtonClick(new EventArgs());

        // if you add an handler it will call it

        testClass.MyButtonClick += myCustomHandler;
        testClass.ButtonClick(new EventArgs()); // myCustomHandler has been invoiked

    }

    private static void myCustomHandler(object sender, EventArgs e)
    {
        Console.WriteLine("myCustomHandler has been invoiked");
    }
}