What is the correct way to Unhook event Handlers in a ViewModel

Question

So I have been doing some research on correctly unhooking event handlers from my view model to prevent Memory leaks.

Say I have a View Model like so:

class MyViewModel
{
    private List<MyObject> _myObjects;
    public List<MyObject> MyObjects
    {
        get { return _myObjects; }
        set { _myObjects = value; }
    }

    public MyViewModel()
    {
        for (int i = 0; i < 10; i++)
        {
            var obj = new MyObject();
            obj.MySampleEvent += Obj_MySampleEvent ;
        }
    }

    private void Obj_MySampleEvent (object sender, EventArgs e)
    {
        //do something
    }
}

Now Initially I found This link Which said Implement IDisposable and add a Dispose Method:

public void Dispose()
{
   foreach (var obj in MyObjects)
   {
       obj.MySampleEvent -= Obj_MySampleEvent;
   }
}

but this wasn't getting called when I would of thought. It seemed to be erratic, Sometimes never even called at all? So I decided to search "When does dispose get called" which lead me to this link explaining that Dispose gets called by the Finaliser / Destructor

Which lead me onto my final piece of research is that I remember people saying do not unhook event Handlers in the Destructor because it will never get called, from this link.

So I just wanted to clarify finally.. What is the correct way of unhooking event handlers in a ViewModel?

i guess this depends on the architecture of your app. My VM's have a clean up function that I use to clean up and it gets called manually when I want and then I loop through all registered viewmodels on application close and clean them up. — J King, Nov 03 '15 at 17:39
Memory leakages? You don't need to unsubscribe. Removing object from the list, *loosing* `_myObject` or instance of `MyViewModel` reference **is sufficient** and will not cause any memory leakages. — Sinatr, Nov 04 '15 at 15:30

score 1 · Accepted Answer · answered Nov 04 '15 at 15:28

1

I usually unregister event handlers on my view-model during navigation.

For example, when the OnNavigatedFrom event is raised (on your view), you can unregister the event handlers on your current view-model. Then when the OnNavigatedTo event is raised, you can re-register the event handlers.

In regards to IDisposable, I am not sure. I thought IDisposable was for managing resources and not business logic.

answered Nov 04 '15 at 15:28

Scott Nimrod

11,206
11
54
118

This makes sense. I suppose `IDisposable` comes about because idealy your view does not want to know about the view model to call `UnregisterEvents` so you need an `Interface` which leads onto there is already an interface `IDisposable` which has a `Dispose` method. So thats probably why people suggest it – JKennedy Nov 05 '15 at 09:30

score 0 · Answer 2 · answered Nov 03 '15 at 18:39

0

Might I suggest converting to an ObservableCollection the reason for the ViewModel is to have something to bind to. You can then subscribe to any events directly on the ViewModel Property which will allow you to subscribe to Window_Closing and cleanup inside that event.

class MyViewModel
{
    private ObservableCollection<MyObject> _myObjects;

    public ObservableCollection<MyObject> MyObjects
    {
        get { return _myObjects; }
        set { _myObjects = value; }
    }

    public MyViewModel()
    {
        for (int i = 0; i < 10; i++)
        {
            var obj = new MyObject();
        }
    }
}

answered Nov 03 '15 at 18:39

Kentonbmax

938
1
10
16

My question VM is just an example to help explain the question. My actual situation is more complex than that so while this is correct I understand the importance of using `ObservableCollection` it actually isn't an answer to the question – JKennedy Nov 04 '15 at 08:37
My question is why are you consuming INotifyPropertyChanged instead of implementing it on your "ViewModel" object? Of course you could implement it and redirect obj.PropertyChanged as well. – Kentonbmax Nov 04 '15 at 13:51
I'm not... it is an example! Let me change the event I am hooking into to `MySampleEvent` to stop the confusion. The point is it doesn't matter what event you hook into, you still need to dispose of it – JKennedy Nov 04 '15 at 15:19
You just answered your own question, and hurt my reputation because your example indicates you are trying to use a view model and do binding. – Kentonbmax Nov 04 '15 at 15:28

What is the correct way to Unhook event Handlers in a ViewModel

2 Answers2