views:

173

answers:

4

When there are no subscribers to an event how do I ensure that an exception will not be thrown if the event is fired.

 // Delegate declaration
 public delegate void _delDisplayChange(object sender,string option);

 // Event declaration
 public event _delDisplayChange  DisplayChange;

 //throwing the event
 DisplayChange(this, "DISTRIBUTION");
+4  A: 

Change this:

// Event declaration      
public event _delDisplayChange  DisplayChange;

to this:

// Event declaration      
public event _delDisplayChange  DisplayChange = delegate{};

This will ensure your event will always have at least one subscriber.

Jay Riggs
No need to add a fictitious subscriber that takes memory and does not obey any logic; checking for null is more consistent with the rationale of event/subscriber logic.
CesarGon
It is an interesting twist, but like Cesar said, there are better ways.
Pierre-Alain Vigeant
What is the "rationale of event/subscriber logic", and why checking for null is consistent with it?
Pavel Minaev
CesarGon
+1 adding an empty subscriber is NOT a dirty trick. This is essentially the null object pattern: you replace the special handling of null by an object (in this case a delegate) which has the same effect. The null object pattern is the more object oriented approach. http://en.wikipedia.org/wiki/Null_Object_pattern
Wim Coenen
+15  A: 

Here is the recommended way to do it:

protected void RaiseDisplayChanged(string message)
{
    var handlers = DisplayChange;
    if(handlers != null)
        handlers(this, message);
}

Copying the event handlers enumeration before checking does two things:

  1. If DisplayChange handlers becomes null between the check and the firing, you don't die
  2. If the listeners modify the DisplayChange list while enumerating through it, you don't run into oddities.

Also, you are not using the standard Event protocol. Your delegate should be:

public delegate void DisplayChangeDelegate(object sender, OptionsEventArgs args);

Where OptionsEventArgs derives from EventArgs. Going a step further, in .Net 3.5, you should never define a delegate like this. Instead, you should just define your event:

public event EventHandler<OptionsEventArgs> DisplayChanged;

I like to take it even one step further by defining this class:

public class EventArgs<T> : EventArgs
{
    public T Payload { get; private set }
    public EventArgs(T payload)
    {
        Payload = payload;
    }
}

Then, you don't need to define OptionsEventArgs:

public event EventHandler<EventArgs<string>> DisplayChanged;

Just some stuff to think about...

Brian Genisio
Thank you for the answer...and the additional information
Brad
Good answer, but it's also worth explaining why the backing field can become `null` (multithreading), and why it's worth making event registration and raising thread-safe to begin with.
Pavel Minaev
+1  A: 

As Brian said: many sources suggest making a copy of the event before checking that it is not null:

_delDisplayChange displayChangeCopy = DisplayChange;
if (displayChangeCopy != null)
    displayChangeCopy(this, "DISTRIBUTION");

This helps to make your code more thread-safe because the value of displayChangeCopy will not change between the null check and the invocation.

antonm
This does NOT make your code thread-safe. It eliminates ONE race condition. It does not eliminate the more general race condition, which is that between the storage of the value of DisplayChange and the invocation of the local copy, *the contents DisplayChange could have changed*, and therefore *you are now invoking a function which might rely upon state that has been destroyed*. Don't go making claims that code is "thread safe" without clearly describing what thread danger you are mitigating. You haven't mitigated nearly all the dangers here.
Eric Lippert
Agreed - I've edited the post to make it more clear.
antonm
A: 

Further to Jay's answer, here's a link regarding performance considerations when using empty delegates.

pFrenchie