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