Why should we use a temprary object for an event?

Question

In most cases, when we use MVVM, we use the INotifyPropertyChanged interface to provide notifications to the bindings, and the general implementation looks like this:

public class MyClass : INotifyPropertyChanged { // properties implementation with RaisePropertyChanged public event PropertyChangedEventHandler PropertyChanged; protected void RaisePropertyChanged(string propertyName) { if (PropertyChanged != null) { PropertyChanged(this, new PropertyChangedEventArgs(propertyName)); } } } 

This works great for me whenever I read code from experts - they write similar code:

 public class MyClass : INotifyPropertyChanged { // properties implementation with RaisePropertyChanged public event PropertyChangedEventHandler PropertyChanged; protected void RaisePropertyChanged(string propertyName) { var tempchanged = PropertyChanged; if (tempchanged != null) { PropertyChanged(this, new PropertyChangedEventArgs(propertyName)); } } } 

I would like to know what exactly is the reason for creating a temporary object for the PropertyChanged event.

Is this just good practice, or are there any other benefits associated with it?

I found an answer with John's answer and an explained example:

Understanding C #: raising events using a temporary variable

Here is a sample code to figure this out:

 using System; using System.Collections.Generic; using System.Threading; class Plane { public event EventHandler Land; protected void OnLand() { if (null != Land) { Land(this, null); } } public void LandThePlane() { OnLand(); } } class Program { static void Main(string[] args) { Plane p = new Plane(); ParameterizedThreadStart start = new ParameterizedThreadStart(Run); Thread thread = new Thread(start); thread.Start(p); while (true) { p.LandThePlane(); } } static void Run(object o) { Plane p = o as Plane; while (p != null) { p.Land += p_Land; p.Land -= p_Land; } } static void p_Land(object sender, EventArgs e) { return; } } 

Answer 1

You are not creating a temporary object. You use a local variable to avoid the race condition.

In this code:

 if (PropertyChanged != null) { PropertyChanged(...); } 

it is possible that PropertyChanged will become null (due to the last subscription of the subscriber) after invalidation, which means that you will get a NullReferenceException .

When you use a local variable, you guarantee that the link that you check for nullity is the same link that you use to create the event, so you will not get an exception. There is still a race condition in which you can end up calling subscribers who simply unsubscribed, but this is inevitable.

Answer 2

This is to avoid the rare case when the last (or only) event handler is removed from the event between the time when you check for null (to see if any event handler is connected) and the time when you trigger the event. If this happens, you will get a NullReferenceException .

If you are concerned about a memory leak - you don't need to - it's just a link, not a copy of an event handler.

More information can be found here.

Answer 3

This is good practice for thread safety reasons.

In the source code, it is theoretically possible for a separate thread to delete the PropertyChanged handler after the if , but before the event was raised on the next line. This will NullReferenceException .

A second pattern removes this risk.

Answer 4

This only matters when working with multi-threaded scripts: when the last event handler is unregistered after checking != null , the actual call can be made with a NullReferenceException in code 1.

However, code 2 does not have this problem, since delegates (the concept of events) are immutable and, therefore, the value of a temporary variable cannot change.

However, I would recommend that you always use option 2 as your best practice - this can save you a headache in the future; -)