I’ve been struck recently by strong parallels between how resource handling and exception safety are usually taught and presented in C++ and in .NET, or more accurately how poorly I think they are often taught.
This post focuses on .Net and the Dispose pattern. I’ll look at C++ in a future post.
What’s Wrong with the .NET Dispose Pattern?
The .Net Dispose Pattern is the standard resource-management pattern for C# and VB programmers, taught as an essential technique to new developers. I think it is unnecessary and promotes bad programming practices.
// Full IDisposable pattern from https://msdn.microsoft.com/en-us/library/fs2xkftw%28v=vs.110%29.aspx // For Reference only - DO NOT USE class BaseClass : IDisposable { bool disposed = false; // Public implementation of Dispose pattern callable by consumers. public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } ~BaseClass() { Dispose(false); } // Protected implementation of Dispose pattern. protected virtual void Dispose(bool disposing) { if (disposed) return; if (disposing) { // Free any other managed objects here. } // Free any unmanaged objects here. disposed = true; } }
This pattern has been taught in C# intro courses and mentioned in blog posts and presentations for years.
I understand why a single comprehensive pattern was developed in the early days of .Net. Many of the Windows developer audience C# was targeted at was were C++ developers and needed to trust that .Net garbage-collection would not be slow, and could still give them deterministic cleanup of other resources. Showing this experienced audience a single named pattern that covered all their concerns was the simplest approach.
On the positive side the Dispose pattern is clear enough, it’s not that hard to follow the pattern, and having a single pattern makes it easier to talk about.
But it pushes bad design, complexity over simplicity and gives new developers a false impression that Dispose is essential for all their classes. In courses and books where new programmers are learning basic syntax and understanding simple constructs like loops and conditions they are shown the Dispose pattern, making it seem that making your classes Disposable is essential (when it’s actually rare), and that this pattern is an example of good programming practice to follow.
Partly this is the same failure of many code examples in early programmer training. Each focuses only on one specific piece of syntax so gives no context to understand how it should be used in practice. Developers are expected to gain experience and gradually gain the understanding to write good, maintainable code using the pieces they have learned.
But I think that common presentation of the details of the IDispose pattern give new developers so many bad habits when it can easily be presented differently and lead new developers to some of the principles of good design at the same time.
Bad design
This pattern suggests that it’s ok to have unmanaged and managed objects in the same class when they should be separated. More generally it suggests it’s ok to have one class responsible for multiple things.
And also that it’s ok to have a class mix layers of abstraction, to have a class responsible for management of low-level resources and high-level functionality acting on those resources.
It also strongly suggests that sub-classing is the commonest method of reuse instead of composition.
Complexity over simplicity
You should only write what you need. You should refactor to the simplest form of what you need.
The whole IDispose pattern is never needed in a good design. Using the whole thing introduces complexity you never should need in one class. Simpler patterns with just the parts needed in common cases would give developers a better idea of good code.
False impression that it’s essential
Intro courses to .Net spend hours describing and implementing it, interviews always include questions about it. New developers are given the impression it’s the most important thing to worry about for each class they write.
So the IDispose pattern has been copied whole into classes over and over in development shops all around the world.
Classes that didn’t need to be IDisposable at all had their design complicated which also complicated the design of classes around them. The vast number of classes which had no need of unmanaged resources had conditions and comments mentioning them to bulk up codebases and confuse future developers.
A Simpler Set of Dispose Patterns
Separating out the cases where IDisposable is and isn’t needed, and showing which are most likely in every day programming makes it clearer what new developers should focus on and help reinforce the idea that each class should be responsible for just one thing.
Default to No Dispose
90% of the classes you write DO NOT NEED IDisposable at all.
They don’t own unmanaged resources.
They refer to a few other managed objects, but only temporarily so they just clean them up as they go, or only borrowed from somewhere else so they are not responsible for cleaning them up.
// Most common case of object that uses but doesn't own other managed IDisposable objects. class ControllerClass { // I'm given this IDisposable object but I didn't create it and I don't have to clean it up, so I don't need a Dispose method. private ProcessorClass processor; public ControllerClass(ProcessorClass processor) { this.processor = processor; } public void DoThings(string filename) { using (var file = File.Open(filename, FileMode.Open)) { //... } // Create and use IDisposable file object in one method, and clean it up straight away, so I don't need a Dispose method. } // No Dispose needed // No Finalizer needed }
Dispose other managed objects
A small number of the classes you write own other managed IDisposable objects.
They don’t own unmanaged resources.
They create and own other managed objects and keep them for their whole lifetime, so they should clean them up.
Leaving the rare unmanaged case out of the dispose pattern for most objects makes it much simpler.
// Simplified Dispose pattern for normal managed object that owns other managed objects. Almost all of your classes needing IDisposable should only need to Dispose other managed objects. class ProcessorClass : IDisposable { // Managed object we own and need to clean up private SpecialFile file; public ProcessorClass() { file = new SpecialFile(); } public void Dispose() { // Dispose any managed objects we own. file.Dispose(); // Normally no reason to keep a bool disposed flag as the file object will do it anyway. // No need to call GC.SuppressFinalize cos we don't have a Finalizer } // No Finalizer needed // So Dispose only run from public Dispose, so no reason having separate internal Dispose implementing fn. }
Special wrappers for unmanaged resources
A tiny number of classes own unmanaged resources. Most .Net developers will never need to write one.
Each unmanaged resource should be wrapped in one managed object, each type of unmanaged resource should be wrapped on one unmanaged class. Most don’t need any work as they already have wrappers (file handles, database connections, network sockets, window handles are already wrapped in managed classes in the .Net Framework Class Library).
Following standard class design advice that a class should have just one job this resource wrapper should only manage the resource, so it has one unmanaged resource and no other managed objects to deal with and no other logic.
// Simplified Dispose for wrapper class for single unmanaged resource. // These are rare as most unmanaged resources already have wrapper classes. If you're writing these often you should rethink your design. class SpecialFile : IDisposable { private int unmanagedHandle; // no need for separate bool disposed flag as handles like most unmanaged resources already have "empty" state. public SpecialFile() { unmanagedHandle = CreateUnmanagedHandle(); } // Clean-up unmanaged resources when user finished. public void Dispose() { DoDispose(); GC.SuppressFinalize(this); } // Clean-up unmanaged resources when garbage-collected if forgotten. ~SpecialFile() { DoDispose(); } // Internal Dispose implementing fn doesn't need to be virtual, better to compose than subclass a SpecialFile. // No bool param needed as only unmanaged resource to clean-up and we always do that whether from Dispose or Finalizer. private void DoDispose() { if (unmanagedHandle == 0) return; FreeUnmanagedHandle(unmanagedHandle); unmanagedHandle = 0; } private static int CreateUnmanagedHandle() { //... } private static void FreeUnmanagedHandle(int unmanagedHandle) { //... } }
Practical Advice for .Net Resource Management
Worrying new developers with the hard, rare cases is unnecessary and shows them bad design, leads new developers to make their designs more complicated than needed, leads them to build classes with too many responsibilities.
So a list of more useful practical advise would be:
- Remember to Dispose files/sockets when you’re done with them, preferably with a “using” statement
- Assume objects injected into your object when it’s constructed will be cleaned-up by whoever created them
- If you create an IDisposable object and keep it for the life of your object then implement IDisposable on your own class and call Dispose on that object from your Dispose
- If you need to use an unmanaged resource
- First check if there is an existing managed class already
- Write a wrapper class that just handles the lifetime of that resource
- (Oh, and there’s an old pattern that handles every option at once, just for reference)
No Comments