Is it good practice to unregister from external events in the Dispose method of an IDiposable class?

Question

I read the excellent answer explaining how to use the Dispose pattern, and also why it works that way.

Proper use of the IDisposable interface

The post clearly states that you would want to use the Dispose pattern in two different scenarios:

1. get rid of unmanaged resources (because we have to)
2. get rid of managed resources (because we want to be helpful)

My question is:

• When an object is subscribed to an external event for its entire lifetime, is it also common/good practice to unregister from that event in the Dispose method? Would you implement the IDisposable interface for that purpose?

Answer 1

Yes, you should.

This is the best way to indicate to consumers of your class that it has "resources" that must be released. (even though event subscriptions are technically not resources)

Answer 2

In many (most?) cases, an object will become eligible for garbage collection very soon after Dispose is called. For example, this will always be true for IDisposable objects instantiated with a using statement:

using(var myDisposableObject = ...)
{
    ...
} // myDisposableObject.Dispose() called here

// myDisposableObject is no longer reachable and hence eligible for garbage collection here

In this situation, I personally wouldn't clutter the code with removal of event subscriptions in the general case.

For example, an ASP.NET Page or UserControl is IDisposable, and often handles events from other controls on the web page. There is no need to remove these event subscriptions when the Page or UserControl is disposed, and in fact I've never seen an ASP.NET application where this is done.

UPDATE

Other answerers suggest you should always unsubscribe to events in the Dispose method of an IDisposable class.

I disagree with this in the general case, though there may be application-specific situations where it is appropriate.

The logical conclusion is that any class that subscribes to events should be IDisposable, so that it can unsubscribe deterministically - I see no logical reason why this recommendation should only apply to classes that happen to own unmanaged resources. I don't think this is a good general recommendation for the following reasons:

• Making a class IDisposable just so it can unsubscribe from events adds complexity for users of the class.

• Unsubscribing from events in the Dispose method requires the developer to keep a track of event subscriptions that need to be removed - somewhat fragile as it's easy to miss one (or for a maintenance developer to add one).

• In situations where a class subscribes to events from a long-lived publisher, it is probably more appropriate to use a Weak Event Pattern to ensure that the subscriber's lifetime is not affected by the event subscription.

• In many situations (e.g. an ASP.NET Page class subscribing to events from its child controls), the lifetime of publisher and subscriber are closely related, so there is no need to unsubscribe.