Recommended pattern for event declaration and handling

Custom event handlers should always follow this pattern:

public delegate void MyCustomEventHandler(object sender, EventArgs e);
public event MyCustomEventHandler OnMyCustomEvent = delegate{};

All I’ve done that is special here is declared the event handler with an empty delegate. This provides two advantages (the big one being the second).

  1. No more need to check for null. Now that we at least have one delegate on the event it will never be null.
  2. This handles the problem of threads unsubscribing from your events. With a check for null you run the risk that even after the check, a different thread might unsubscribe and now the event is null.

As a clearer, more complete example imagine we have the following code:

public delegate void FilesReadyToBeOpenedHandler(object sender, FilesReadyToBeOpenedEventArgs e);
public event FilesReadyToBeOpenedHandler OnFilesReadyToBeOpened;
public class FilesReadyToBeOpenedEventArgs : EventArgs
{
	public string[] FilePaths { get; private set; }
	public FilesReadyToBeOpenedEventArgs(string[] filePaths)
	{
		FilePaths = filePaths;
	}
}
private void btnOpenFiles_Click(object sender, RoutedEventArgs e)
{
	OpenFileDialog ofd = new OpenFileDialog();
	ofd.Multiselect = true;
	ofd.CheckFileExists = true;
	if(ofd.ShowDialog() == false || ofd.SafeFileNames.Length == 0)
		return;
		
	FilesReadyToBeOpenedEventArgs readyFilesEventArgs = new FilesReadyToBeOpenedEventArgs(ofd.SafeFileNames);

	if (OnFilesReadyToBeOpened != null)
	{
		//we've checked for null but what happens if another thread unsubscribes at this moment?
		OnFilesReadyToBeOpened(this, readyFilesEventArgs);
	}
}

but just by modifying the event declaration to:

public event FilesReadyToBeOpenedHandler OnFilesReadyToBeOpened = delegate { };

you now have safer, cleaner and easier to maintain code. My button click event therefore changes to:

FilesReadyToBeOpenedEventArgs readyFilesEventArgs = new FilesReadyToBeOpenedEventArgs(ofd.SafeFileNames);
OnFilesReadyToBeOpened(this, readyFilesEventArgs);

Thanks,
Brian

Leave a Reply