Is there a downside to adding an anonymous empty d

2019-01-02 17:38发布

I have seen a few mentions of this idiom (including on SO):

// Deliberately empty subscriber
public event EventHandler AskQuestion = delegate {};

The upside is clear - it avoids the need to check for null before raising the event.

However, I am keen to understand if there are any downsides. For example, is it something that is in widespread use and is transparent enough that it won't cause a maintenance headache? Is there any appreciable performance hit of the empty event subscriber call?

9条回答
后来的你喜欢了谁
2楼-- · 2019-01-02 17:55

For systems that make heavy use of events and are performance-critical, you will definitely want to at least consider not doing this. The cost for raising an event with an empty delegate is roughly twice that for raising it with a null check first.

Here are some figures running benchmarks on my machine:

For 50000000 iterations . . .
No null check (empty delegate attached): 530ms
With null check (no delegates attached): 249ms
With null check (with delegate attached): 452ms

And here is the code I used to get these figures:

using System;
using System.Diagnostics;

namespace ConsoleApplication1
{
    class Program
    {
        public event EventHandler<EventArgs> EventWithDelegate = delegate { };
        public event EventHandler<EventArgs> EventWithoutDelegate;

        static void Main(string[] args)
        {
            //warm up
            new Program().DoTimings(false);
            //do it for real
            new Program().DoTimings(true);

            Console.WriteLine("Done");
            Console.ReadKey();
        }

        private void DoTimings(bool output)
        {
            const int iterations = 50000000;

            if (output)
            {
                Console.WriteLine("For {0} iterations . . .", iterations);
            }

            //with anonymous delegate attached to avoid null checks
            var stopWatch = Stopwatch.StartNew();

            for (var i = 0; i < iterations; ++i)
            {
                RaiseWithAnonDelegate();
            }

            stopWatch.Stop();

            if (output)
            {
                Console.WriteLine("No null check (empty delegate attached): {0}ms", stopWatch.ElapsedMilliseconds);
            }


            //without any delegates attached (null check required)
            stopWatch = Stopwatch.StartNew();

            for (var i = 0; i < iterations; ++i)
            {
                RaiseWithoutAnonDelegate();
            }

            stopWatch.Stop();

            if (output)
            {
                Console.WriteLine("With null check (no delegates attached): {0}ms", stopWatch.ElapsedMilliseconds);
            }


            //attach delegate
            EventWithoutDelegate += delegate { };


            //with delegate attached (null check still performed)
            stopWatch = Stopwatch.StartNew();

            for (var i = 0; i < iterations; ++i)
            {
                RaiseWithoutAnonDelegate();
            }

            stopWatch.Stop();

            if (output)
            {
                Console.WriteLine("With null check (with delegate attached): {0}ms", stopWatch.ElapsedMilliseconds);
            }
        }

        private void RaiseWithAnonDelegate()
        {
            EventWithDelegate(this, EventArgs.Empty);
        }

        private void RaiseWithoutAnonDelegate()
        {
            var handler = EventWithoutDelegate;

            if (handler != null)
            {
                handler(this, EventArgs.Empty);
            }
        }
    }
}
查看更多
只靠听说
3楼-- · 2019-01-02 17:55

Instead of "empty delegate" approach one can define a simple extension method to encapsulate the conventional method of checking event handler against null. It is described here and here.

查看更多
千与千寻千般痛.
4楼-- · 2019-01-02 17:59

I would say it's a bit of a dangerous construct, because it tempts you to do something like :

MyEvent(this, EventArgs.Empty);

If the client throws an exception, the server goes with it.

So then, maybe you do:

try
{
  MyEvent(this, EventArgs.Empty);
}
catch
{
}

But, if you have multiple subscribers and one subscriber throws an exception, what happens to the other subscribers?

To that end, I've been using some static helper methods that do the null check and swallows any exception from the subscriber side (this is from idesign).

// Usage
EventHelper.Fire(MyEvent, this, EventArgs.Empty);


public static void Fire(EventHandler del, object sender, EventArgs e)
{
    UnsafeFire(del, sender, e);
}
private static void UnsafeFire(Delegate del, params object[] args)
{
    if (del == null)
    {
        return;
    }
    Delegate[] delegates = del.GetInvocationList();

    foreach (Delegate sink in delegates)
    {
        try
        {
            sink.DynamicInvoke(args);
        }
        catch
        { }
    }
}
查看更多
孤独总比滥情好
5楼-- · 2019-01-02 18:01

The only downside is a very slight performance penalty as you are calling extra empty delegate. Other than that there is no maintenance penalty or other drawback.

查看更多
有味是清欢
6楼-- · 2019-01-02 18:01

It is my understanding that the empty delegate is thread safe, whereas the null check is not.

查看更多
何处买醉
7楼-- · 2019-01-02 18:06

Instead of inducing performance overhead, why not use an extension method to alleviate both problems:

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

Once defined, you never have to do another null event check again:

// Works, even for null events.
MyButtonClick.Raise(this, EventArgs.Empty);
查看更多
登录 后发表回答