views:

1014

answers:

2

When implementing IDisposable correctly, most implementations, including the framework guidelines, suggest including a private bool disposed; member in order to safely allow multiple calls to Dispose(), Dispose(bool) as well as to throw ObjectDisposedException when appropriate.

This works fine for a single class. However, when you subclass from your disposable resource, and a subclass contains its own native resources and unique methods, things get a little bit tricky. Most samples show how to override Dipose(bool disposing) correctly, but do not go beyond that to handling ObjectDisposedException.

There are two questions that I have in this situation.


First:

The subclass and the base class both need to be able to track the state of disposal. There are a couple of main options I know of -

  • 1) Declare private bool disposed; in both classes. Each class tracks its own this.disposed, and throws as needed.

  • 2) Use protected bool Disposed { get; private set; } instead of a field. This would let the subclass check the disposed state.

  • 3) Provide some protected helper method to check the disposed state, and throw by pulling the current type name via reflection if the object is disposed.

The advantages as disadvantages I see to each by option are:

  • 1) This "smells" to me since it contains duplicated booleans, but seems to work fine. I often use this when subclassing other code.

  • 2) This takes out the duplicated booleans, but is not the way the design guidelines books are written, etc. This is what I typically use, though, since it keeps it a single point for state.

  • 3) This seems like the cleanest option to me, but doesn't appear in standard guidelines. It may be a little less expected of an approach than others from users of the class.

I, at one point or another, have tried using all three of these approaches. I would like to know advantages and disadvantages to the three approaches, as well as any other ideas for a cleaner, better way to handle this. What choice would you make in handling this, and why?


Second:

When throwing the ObjectDisposedException, what do you use for the name argument? I know the "typical" method call is:

throw new ObjectDisposedException(GetType().FullName);

There is a comment on this page from a Microsoft employee suggesting that implementing the concrete class's full name is the appropriate usage.

In the third option above, this would be the only meaningful choice. However, if the class implements the throwing itself, you could potentially return the name of the class that defines the method that was called. (ie: the base class could return the base class's name, not the concrete subclass)

I don't think this is a good idea - but I ran into this on some code written by somebody else. Are there advantages or disadvantages to having the name of the class implementing the method returned?

+1  A: 

I typically implement the first option. Indeed, it seems to be what the design guidelines recommend. The reason isn't immediately apparent, but I consider it a good one nonetheless: any implementer of the class should have the same sort of protection against the case where the object is disposed as general consumers. In other words, it's best not to assume that whoever is implementing a derived class knows precisely when they can call a certain method, whose successful execution may or may not depend on whether the object has already been disposed or not (though ideally this should be documented via XML comments anyway).

Regarding your second question, I would again stick with the recommended practice of passing GetType().FullName, especially since it's used in the core .NET framework. Even if you think alternative methods are more appropiate, I think it's best to stick to the method used in the .NET framework for the sake of consistency.

To conclude: as with all guidelines, it's clearly up to you how you want to implement a certain design feature, though unless you have a particularly good reason it's highly advisable just to stick with them. In both these situations, it probably wouldn't do a great deal of harm to utilise some of the alternatives you suggested in your post, so long as they are used consistently and preferably documented to the user.

Noldorin
A: 

Declare private bool disposed; in both classes. Each class tracks its own this.disposed, and throws as needed.

It is the practical solution when you are unable to modify the base class.

Use protected bool Disposed { get; private set; } instead of a field. This would let the subclass check the disposed state.

Why not make it public and call it IsDisposed instead? Then you would be doing the same thing as System.Windows.Forms.Control. This is a good solution when you can modify the base class.

you could potentially return the name of the class that defines the method

No. The example code you referenced used "GetType().FullName". This is always the name of the most derived type, not the type that implements the particular method.

binarycoder
GetType().FullName will always return the concrete type. I was saying that if you implement bool disposed in each class, individually, you don't HAVE to do that - you can return something else (like the name of the class containing the method).
Reed Copsey