C#: Initializing an event handler with a dummy

Question

I've seen this sort of code some places:

public event SomeEventHandler SomeEvent = (s, e) => { };

Is that a recommended way of doing things? What does it solve, and does it have any noteworthy side effects? Will I still have to do null checks? Or is that exactly what I don't have to do any more? Will garbage collection still work as it should?

---

For example:

private PropertyChangedEventHandler propertyChanged;
private readonly object propertyChangedLock = new object();
public event PropertyChangedEventHandler PropertyChanged
{
    add
    {
        lock (propertyChangedLock)
            propertyChanged += value;
    }
    remove
    {
        lock (propertyChanged)
            propertyChanged -= value;
    }
}
protected void OnPropertyChanged(string propertyName)
{
    PropertyChangedEventHandler handler;
    lock (propertyChangedLock)
        handler = propertyChanged;

    if (handler != null)
        handler(this, new PropertyChangedEventArgs(propertyName));
}

Could I change the first line into this:

private PropertyChangedEventHandler propertyChanged = (s, e) => { };

And then skip the null-check in the OnPropertyChanged method? And if I then skip the null-check could I then also skip the lock? If so that would give me this:

protected void OnPropertyChanged(string propertyName)
{
    propertyChanged(this, new PropertyChangedEventArgs(propertyName));
}

Would that be safe when taking the initialization into account? Or are there some side effects I have missed?

Answer 1

You can see it as an implementation of the NULL Object pattern.

It helps making your code more readable, since you do not need to do NULL - value checks.

The locks in your add / remove logic will have to remain, if they're necessary now. They have nothing to do with it. They're used to avoid race-conditions (but i don't know if they're necessary in your very situation)

Answer 2

While you don't need to do the nullity checks, if you really want to try to make the event thread-safe, you still need to fetch it in a lock:

protected void OnPropertyChanged(string propertyName)
{
    PropertyChangedEventHandler handler;
    lock (propertyChangedLock)
    {
        handler = propertyChanged;
    }
    handler(this, new PropertyChangedEventArgs(propertyName));
}

Otherwise you may not be fetching the most recent value - if event handlers are being added in a different thread, you could theoretically raise events forever without ever calling the new handlers. In practice I believe you'll almost always get away without the lock, but in memory-model terms you should have some sort of fence.

Personally I recommend that you don't try to make the events thread-safe.