EventHandler is null when trying to fire event in WPF

Question

Issue

---

I have created an event in the class LoginVM which looks like the following:

public class LoginVM : INotifyPropertyChanged
{
    public event EventHandler<string> PasswordSet;
}

Also in this class I have a piece of code which fires this event:

public class LoginVM : INotifyPropertyChanged
{
    public event EventHandler<string> PasswordSet;

    private void PopulateLatestServer()
    {
        try
        {
            string SERVER_ID = Registry.GetValue(@"HKEY_CURRENT_USER\SOFTWARE\PODIA", "LATESTSERVER", null).ToString();

            BDO_SERVERS latestserver = SavedServers.Where(a => a.Server_ID == SERVER_ID).FirstOrDefault();

            setServerURL(latestserver.ServerURL, false);
            Username = latestserver.Username;
            PasswordSet(this, latestserver.Password);
        }
        catch (Exception)
        {
            Global.WriteLog("Could not find last logged in server.", EventLogEntryType.Warning);
        } 
    }
}

I have another class which is called LoginV and in there I create an instance of the class and subscribe to the event:

public partial class LoginV : MetroWindow
{
    public LoginV()
    {
        InitializeComponent();

        LoginVM _loginVM = new LoginVM();
        this.DataContext = _loginVM;
        _loginVM.PasswordSet += new EventHandler<string> (_loginVM_PasswordSet);

    }

    private void _loginVM_PasswordSet(object sender, string e)
    {
        passwordBox.Password = e;
    }

As you can probably tell I am trying to trigger an event from the ViewModel to the View but every time I trigger the event from the ViewModel, PasswordSet is null and errors.

Answer 1

It's a good idea to initialize the password in the constructor for LoginVM as you did. That's when initialization ought to happen. Ordinarily, you'd set a property and the binding in the XAML would take care of updating the control. No need for an event on the VM. But this is a password box, so you can't bind it, and the event you wrote is The Right Thing.

But in your implementation, that leaves you with this sequence of events:

1. Create VM
2. VM raises PasswordSet in its constructor -- without checking to see if there are any handlers.
3. View assigns VM to DataContext
4. View adds handler to PasswordSet event

And you get an exception at step 2, because you didn't check for handlers.

Here's what you do.

In the VM or anywhere, always use this pattern for raising events:

C# <= 5:

protected void OnPasswordSet(String e)
{
    var handler = PasswordSet;

    if (handler != null)
    {
        handler(this, e);
    }
}

C#6

protected void OnPasswordSet(String e) => PasswordSet?.Invoke(this, e);

Either:

private void PopulateLatestServer()
{
    try
    {
        string SERVER_ID = Registry.GetValue(@"HKEY_CURRENT_USER\SOFTWARE\PODIA", "LATESTSERVER", null).ToString();

        BDO_SERVERS latestserver = SavedServers.Where(a => a.Server_ID == SERVER_ID).FirstOrDefault();

        setServerURL(latestserver.ServerURL, false);
        Username = latestserver.Username;
        OnPasswordSet(latestserver.Password);
    }
    catch (Exception)
    {
        Global.WriteLog("Could not find last logged in server.", EventLogEntryType.Warning);
    } 
}

Can't crash now. Or at least not the same way as last time.

Problem number two: How do you update the view initially?

Easy: Take whatever's in the view's PasswordSet handler, move it into a protected method, and call that in both places. This looks a little verbose since it's only a one-liner, but it's nice to have things rolled into neatly labeled units. If that code were more complicated, you'd absolutely want not to be copying and pasting it. If it gets more complicated a year from now, you won't have to waste any time re-parsing your the old code.

public partial class LoginV : MetroWindow
{
    public LoginV()
    {
        InitializeComponent();

        LoginVM _loginVM = new LoginVM();
        this.DataContext = _loginVM;
        _loginVM.PasswordSet += new EventHandler<string> (_loginVM_PasswordSet);

        UpdatePassword();
    }

    protected void UpdatePassword()
    {
        passwordBox.Password = e;
    }

    private void _loginVM_PasswordSet(object sender, string e)
    {
        UpdatePassword();
    }

Option number two: Keep OnPasswordSet() as shown above, but instead of having the view manually update the password in the constructor, have the LoginVM require a PasswordSet handler as a parameter. This isn't the way I would do it; constructor parameters like this get on my nerves. But that may just be an irrational prejudice on my part. This way makes more clear the fact that the owner needs to handle that event to use the class, and "provide a suitable event handler" becomes the only thing the consumer needs to do in order to use the thing. The less a consumer needs to know about your class's internals, the better, for obvious reasons. Platonically ideal design would be when programmers who don't think at all can make casual glib assumptions about your class, and not end up on Stack Overflow begging somebody to read the documentation to them out loud. We'll never get there, though.

public class LoginVM : INotifyPropertyChanged
{
    public LoginVM(EventHandler<string> passwordSetHandler)
    {
        if (passwordSetHandler != null)
        {
            PasswordSet += passwordSetHandler;
        }

        PopulateLatestServer();
    }

    //  If the consumer doesn't want to handle it right way, don't force the issue.
    public LoginVM()
    {
        PopulateLatestServer();
    }

A third option is to set up explicit add/removes for the event, and raise the event when the handler comes in:

public class LoginVM : INotifyPropertyChanged
{
    private event EventHandler<string> _passwordSet;
    public event EventHandler<string> PasswordSet
    {
        add
        {
            _passwordSet += value;
            //  ...or else save latestServer in a private field, so here you can call 
            //  OnPasswordSet(_latestServer.Password) -- but since it's a password, 
            //  best not to keep it hanging around. 
            PopulateLatestServer();
        }
        remove { _passwordSet -= value; }
    }

Answer 2

An event is null when there's no listener to the event.

private void RaisePasswordSet(String pass) {
    YourEventArgs args = new YourEventArgs(pass);
    if(PasswordSet != null) PasswordSet(this, args);
}

Your issue is that when you try to raise the event no one listen to it yet.