views:

93

answers:

2

An stock data download class has a BarList to which it either adds new bars or updates and replaces the last bar when the last bar changes realtime. Whenever this download class adds a new bar to the BarList class or changes its last bar, it also calls its NotifyOnBarsAdded or NotifyOnBarChanges. I'm trying to get the notify methods to raise events so that the Canvas class which handles these events can redraw the last bar or entire chart depending on which notify method is called. The problem is that when the NotifyOnBarsAdded class is called i get a NullReferenceException as try to raise the event. I am raising the event like this: NotifyBarAdded(this, EventArgs.Empty). Is this incorrect? Here's the code:

public class BarList : List< Bar >
{
    private int historyHandle;
    public event EventHandler NotifyBarChanged;
    public event EventHandler NotifyBarAdded;

    public BarList(int historyHandle)
    {
        this.historyHandle = historyHandle;
    }

    public BarList()
    {
        // TODO: Complete member initialization
    }

    public void NotifyOnBarChange()
    {
        NotifyBarChanged(this,EventArgs.Empty);
    }

    public void NotifyOnBarsAdded()
    {
         NotifyBarAdded(this, EventArgs.Empty);
    }

    public long handle { get; set; }


}
+2  A: 

You always need to check to see if someone has actually hooked onto an event

 public void NotifyOnBarChange()
    {   
        if (NotifyBarChanged != null)
                 NotifyBarChanged(this,EventArgs.Empty);
    }

They will take your object and call something like:

yourObject.NotifyBarChanged  += ...their event handler ...;
Preet Sangha
Note that this method is not thread-safe. If your event could be accessed from multiple threads, you should use Brian's method (http://stackoverflow.com/questions/4039790/how-do-you-raise-an-event-in-c/4039815#4039815).
Gabe
Brian's method is unfortunately not thread-safe, although it looks like the intent is there.
Ben Voigt
... and now Brian fixed the race condition.
Ben Voigt
yes I know - I was being lazy!
Preet Sangha
+5  A: 

What you're doing is correct, except you have to take into account that there could be no event handlers attached, which will give you a NullReferenceException. You either have to insert a null guard before calling or initialize the events with an empty delegate like this.

Checking for null:

var local = NotifyBarAdded;
if(local != null) local(this, EventArgs.Empty);

Initializing with an empty delegate will allow you to keep the calls you already have (i.e. no need to check for null).

public event EventHandler NotifyBarAdded = delegate {};
Brian Rasmussen
You might as well use `local` to call the handlers as well, that will actually eliminate a race condition.
Ben Voigt
@Ben: Of course. Too early in the morning for me. Thanks for pointing that out.
Brian Rasmussen