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).
- No more need to check for null. Now that we at least have one delegate on the event it will never be null.
- 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
