2

In my application I have simple EventHandler (in my ViewModel) defined as this :

public event EventHandler FavouriteChangedEvent;

Then when I need, I fire the event with this

FavouriteChangedEvent(this, null);

Everything is working as expected, however in my app, there are clickable links, which opens webbrowser (as separate app). When you go back to the application, then when I try to run FavouriteChangedEvent(this, null); it ends with NullPointerException (From debugging it is beacause FavouriteChangedEvent is really null).

Why is that?

I find this on the internet and used it

    public delegate void EventHandler(object sender, string myValue);
    public event EventHandler FavouriteChangedEvent = delegate { }; 

But it does not help much. The application does not fall, but in my View class, I have this line in constructor _dataContext.FavouriteChangedEvent += _dataContext_FavouriteChangedEvent;

After go out and back to the app, this event is not triggered anymore.

libik
  • 22,239
  • 9
  • 44
  • 87
  • You should unsubscribe from event in the view when it's destroyed. If you don't, then view is not destroyed (memory leak) and it still trying to handle event, perhaps throwing exception therefore your new view doesn't get notified about event anymore. – Sinatr Jul 01 '15 at 08:42
  • @Sinatr - no, the null pointer is thrown in the ViewModel class itself. No matter if something does subscribe or not. And I dont know why :). – libik Jul 01 '15 at 09:13
  • Well, **it can't be `null`**, because you set its initial value as `delegate { }` and you don't unsubscribe from that, right? Either your debugging or some other conclusion is wrong. Btw, check out [this answer](http://stackoverflow.com/a/30983626/1997232) regarding events (you don't need delegate there, simply use `EventHandler<>`, moreover see `EventArgs` part). – Sinatr Jul 01 '15 at 09:19
  • @Sinatr - if I do it with the second approach, it is not null. However in first case it is null and I do not know why. – libik Jul 01 '15 at 10:55

1 Answers1

2

Then when I need, I fire the event with this

FavouriteChangedEvent(this, null);

it ends with NullPointerException (From debugging it is beacause FavouriteChangedEvent is really null

This is because there are no event handlers yet.

The event is a class

Once a class has declared an event, it can treat that event just like a field of the indicated delegate type. The field will either be null, if no client has hooked up a delegate to the event, or else it refers to a delegate that should be called when the event is invoked. Thus, invoking an event is generally done by first checking for null and then calling the event.

You have to at least check for null before rising event:

if(FavouriteChangedEvent != null)
    FavouriteChangedEvent(this, null);

Note, to make this thread-safe you will have to copy event to local variable, to ensure what null check and rising occurs for the same event:

var event = FavouriteChangedEvent;
if(event != null)
    event(this, null);

Described solution (with attaching empty delegate on constructing) is another possible thread-safe solution.

According to event design guidelines:

DO use a protected virtual method to raise each event

This said - do not rise event handler like this, but call protected virtual method (called OnFavouriteChangedEvent) which will rise event. And there is no need to include word "event" into event name. Make it simple: FavouriteChanged.

Community
  • 1
  • 1
Sinatr
  • 20,892
  • 15
  • 90
  • 319