Setting/Removing event handlers in .Net

Question

So I am stuck with fixing/maintaining another programmers code (blech)

I am a firm professor of the rule "If it ain't broke dont fix it!" so depsite wanting to change something every time I come across horrendous code, I am keeping myself limited to only changing the absolute minimum amount of code possible to make the required fixes. But in some cases I really need to understand something before trying to follow it/change it.

I came across this little bit here:

region.LineSelected = (x) => { };

And am wondering if it's the same as this:

region.LineSelected = null;

I want to be 100% positive of what the first line is doing before I go changing the method it's in.

Answer 1

Edit based on my current opinion on the subject

They are not the same. The lambda version is adding an event handler to an empty anonymous method. This allows other code to freely raise LineSelected() without worrying about whether it is null (ie. having no listeners).

Eg.

var lineSelected = this.LineSelected;

if (lineSelected != null)
{
    lineSelected(EventArgs.Empty);
}

The above statement can throw a NullReferenceException if something unsubscribes from LineSelected in another thread after the if but before the event is raised. Assigning LineSelected to a temporary variable and then raising that can call an unsubscribed event listener. Assigning the event handler to a local variable is the recommended method of handling null delegates.

By adding an empty delegate, other code is always able to call LineSelected without fear of a NullReferenceException. By assigning the multicast event delegates to a local variable, you can be sure that the value cannot be modified by another thread.

Answer 2

No it is not the same thing - the first line assigns LineSelected an empty delegate which is very different from null.

The easiest way to spot the difference is to look at the code that compiler generates on your behalf when you use the lambda syntax. This code:

using System;

class Program
{
    static void Main()
    {
     Action<int> func0 = (x) => { };
     Action<int> func1 = null;
    }
}

Really compiles to this:

internal class Program
{
    // Methods
    private static void Main()
    {
        Action<int> func0 = delegate (int x) {
        };
    }
}

Notice that the compiler was smart enough to remove func1 as it was set to null and not referenced elsewhere. But notice that func0 still remains and is set to a delegate that albeit does nothing but is very different from null.

Answer 3

I can't think of any reason to have that first line of code. The only thing I can think of, is when raising the LineSelected event, if you have the first line of code in the class, you don't need to check if the LineSelected event is null. ie:

if (this.LineSelected != null)
{
   LineSelected(this,new EventArgs());
}

Instead, you can just raise the event without null checks.

However, with the second line of code, you will need to check for nulls.

Answer 4

They are not the same, because the event handler is set.

Lets say the class that exposes the LineSelected, forgot to:

if( LineSelected != null )
    LineSelected(...)

If that class were to call the LineSelected and no one is listening, then it will throw NullReferenceException

Note that you can also do (inside region) to avoid the race condition:

var event = LineSelected; if( event != null ) event(...

Answer 5

I think this a technique to avoid null checks on every event.

If the LineSelected event-raising code does not have a proper null check, then this will cause an exception:

region.LineSelected = null;

/* no event handlers added to LineSelected */

class Region {
    void OnLineSelected() {
        // Null error!
        LineSelected();
    }
}

However, if there is an empty no-side effect handler added to it, then the above code will work just fine, even if nobody adds a handler to the event, because there will always be that empty handler attached.

Answer 6

To expand on what Richard and Andrew said, it's the equivalent of

region.LineSelected = delegate {};

Which means that when you raise the event, you don't need to check for null first, because it has a delegate (at the price of a small performance hit)

Answer 7

That's not an event handler, that's a simple delegate. (An event handler would have to be modified with += and -= to attach and detach the event).

As has been commented before, setting the delegate property to an empty handler means that null checks don't have to be performed before invoking the delegate (assuming that nothing else can set it to null).

This may make calling code more convenient, but it should not be confused as being a performance improvement (by removing the need to check for null). Generally the overhead of a delegate method call will be significantly higher than that of the null check. There may be some scenarios (for example if the delegate has a "real" implementation 99.99% of the time) where avoiding the null check could improve performance but it's hard to imagine a scenario where the tiny bit of performance difference there could matter enough to be worth it that wouldn't also warrant removing the delegate invocation entirely in favor of something more efficient.