5

E.g. for a general type, which subscribe to some events in constructor:

class SomeType
{
    public SomeType(...)
    {
        someEvent1 += ...
        someEvent2 += ...
    }
}

Where do I unsubscribe from events?

  • Finalizer?
  • IDisposable ?
  • Some method DontForgetToCallMeSoICanUnsubscribeFromEvents()?
  • Use weak events pattern?

I know it depends. In case of controls (wpf, winforms) there are some events what can be used to subscribe/unsubscribe like Loaded/Unloaded, HandleCreated/HandleDestroyed, etc. But what if parent is a simple object?

And some more specific example: nested ViewModels, where each level is a List<NextLevelVM>, at any level ViewModel can be deleted, does that means what each ViewModel must implement IDisposable (if e.g. it is the right way) where it call Dispose for each item in their list? I tried to use weak events, but that doesn't go well.

Community
  • 1
  • 1
Sinatr
  • 20,892
  • 15
  • 90
  • 319
  • @JamesThorpe, event source lives longer. Maybe as long as application. And you (or me?) don't want memory leakage (if `SomeType` is an item in the big list). – Sinatr Apr 21 '16 at 12:02
  • Your code is missing details -- what object does `someEvent1` belong to? Presumably not `this` (subscribing to your own events is, well, silly). Are you subscribing to the events of an object passed in the constructor as a parameter (the "parent")? – Jeroen Mostert Apr 21 '16 at 12:12
  • @JeroenMostert, is it too broad to cover any scenario? What if I say what `someEvent1` is event of instance passed as constructor parameter and `someEvent2` is a static event. What would be the difference? – Sinatr Apr 21 '16 at 12:20
  • I am looking for an answer which takes *the best* option and explains why others are not so good. It might be what the best option is not listed. – Sinatr Apr 21 '16 at 12:21
  • What was the result when you tried removing the event handlers in Dispose()? – Brian Pursley Apr 21 '16 at 12:21
  • What would be the difference? *It depends*. Typically, you make unsubscribing a non-issue by either coupling the lifetimes of the publisher and the subscriber (so you don't need to bother) or you use weak events so the subscriber isn't kept alive by the publisher. You seem to be putting the cart before the horse -- events are a mechanism, not the goal. There is no general answer to "where do I unsubscribe" any more than there is one to "where do I remove objects from this list I've been adding objects to". Events *are*, in fact, just lists of delegates, so maybe that analogy helps. – Jeroen Mostert Apr 21 '16 at 12:35
  • @bpursley, I haven't tried it yet. But that has following disadvantages (compared to weak events): 1) you must implement `IDisposable` 2) ViewModels have to implement it 3) the highest level should never miss `Dispose()` call. But an andvantage (compared to weak events) if that you don't have to check in event handler if your object is still alive (because it may not be, but event handler will still called until GC occurs). – Sinatr Apr 21 '16 at 12:35
  • 2
    @Sinatr can you explain the difference between pt 1 and 2. To me it sounds same - viewmodel/whoever subscribes to event should implement IDisposable. pt 3) if you are not calling dispose it will be caught by the Static code analysis if you are paying attention. To me implementing IDisposable would be the right approach and these points are really not a problem. – Carbine Apr 21 '16 at 12:41
  • @CarbineCoder, option (2) should be "ViewModels have to know about it". When you (typically) have a list of viewmodels you would never expect they impliment `IDisposable`. What I mean in (2) is what ViewModel user has to be aware what suddenly he must `Dispose()` something (this is unusual). – Sinatr Apr 22 '16 at 06:49

1 Answers1

0

I've found a really good way to handle this issue is to create two methods in the page's code behind, that calls methods on your ViewModel to start/stop listening to events depending on whether it's visible or not.

Below I'm using the Appearing functions, but depending on the framework you're using it might be slightly different, but the strategy should work.

In Page class:

protected override void OnAppearing()
{
    base.OnAppearing();
    _myViewModel.StartListeningToEvents();
}

protected override void OnDisappearing()
{
    base.OnDisappearing();
    _myViewModel.StopListeningToEvents();
}

Then in my ViewModel, I actually subscribe to the events I require:

public void StartListeningToEvents()
{
    SomeProperty.PropertyChanged += PropertyUpdated;
}

public void StopListeningToEvents()
{
    SomeProperty.PropertyChanged -= PropertyUpdated;
}

void PropertyUpdated(object sender, PropertyChangedEventArgs e)
{
    // do stuff
}

My example is for a property change event. But similar code should work for any event.

In this way you're guaranteed that your page is only listening to events when it's open, and you don't need to worry about disposing anything besides calling the one event when the page is no longer open.

Adam B
  • 3,662
  • 2
  • 24
  • 33