Is unsubscribing from event necessary for an object with shorter lifetime?

Question

I have a view model which retrieves an object from some service, and makes it available for data binding. The object is implementing INotifyPropertyChanged. In the view model, I am listening to the PropertyChanged event to perform some internal actions when certain properties in the object are modified.

Now it is possible that a new object is requested from the service, completely replacing the old object. Given that the lifetime is essentially limited by the view model itself, and nobody else holds a reference to it (WPF uses weak listeners), do I need to unsubscribe from the object in this case? Of course, I should and it’s simple enough to do so in the setter, but do I really need to?

public class MyViewModel : INotifyPropertyChanged
{
    private DataType myData;
    public DataType MyData
    {
        get { return myData; }
        protected set
        {
            if (value == myData)
                return;

            if (myData != null)
                myData.PropertyChanged -= DataPropertyChanged;
            myData = value;
            myData.PropertyChanged += DataPropertyChanged;

            OnNotifyPropertyChanged("MyData");
        }
    }

    public void UpdateData ()
    {
        MyData = service.GetData();
    }

    // ...
}

*[...](WPF uses weak listeners)[...]* - Can you give a reference for this? — DHN, Jun 19 '13 at 13:06
[`WeakEventManager`](http://msdn.microsoft.com/en-us/library/system.windows.weakeventmanager.aspx) allows the subscription of weak events, but I don't think the manual subscription `someEvent += SomeDelegate` is overridden. If event subscription/unsubscription is deterministic then stick with simplicity. — Dustin Kingen, Jun 19 '13 at 13:12
@DHN WPF’s binding manager uses the [`PropertyChangedEventManager`](http://msdn.microsoft.com/en-us/library/system.componentmodel.propertychangedeventmanager.aspx). It’s also mentioned on [MSDN’s topic about weak events](http://msdn.microsoft.com/en-us/library/aa970850.aspx). And lastly, there is [this answer](http://stackoverflow.com/a/3713307/216074) :) — poke, Jun 19 '13 at 13:14
For cleanness, I'd implement IDisposable and unsubscribe from the event in Dispose, as you still have a double reference when you're done with MyViewModel, which is only going to be an issue if there's a reason to shut down MyViewModel without shutting down the whole process. Most code reviewers will insist on the 'myData.PropertyChanged -= DataPropertyChanged;' in the setter, but it's not really necessary unless you need to speed up garbage collection. — Denise Skidmore, Jun 20 '13 at 13:39

score 2 · Accepted Answer · answered Jun 21 '13 at 13:34

You don't really need to do anything, but you should detach the old object from the event when you're done. For two reasons.

If the object is garbage collected and the event is fired, some time is going to be spent figuring out the object is no longer alive. Hopefully it will then be removed from the event handler list. If not, some more time is going to be spent the next time, too, and so on.

More importantly, if your old object is not garbage collected and the event is fired, you're going to get two event notifications - once in the old object and once in the new one. You will need to handle this case specifically in the old object (or else, bad things will happen).

Easiest way to handle this is to detach from the event when you're done.

Is unsubscribing from event necessary for an object with shorter lifetime?

1 Answers1