views:

179

answers:

3

I am working with Active Directory using C#. Instantiating the PrincipalContext object seems to be expensive, so I'd like to store one in a class variable.

When using PrincipalContext as a local variable, I can use the convenient using syntax. When storing an IDisposable object in a static variable, how do I ensure the object is properly disposed of?

+4  A: 

The general pattern for this is to implement the IDisposable interface on your class. Take this example:

public class YourClass : IDisposable
{
    private OtherDisposableType yourResource;

    public YourClass()
    {
        yourResource = new OtherDisposableType();
    }

    public void Dispose()
    {
        yourResource.Dispose();
    }
}

This is, at a minimum, what you need to do.

EDIT

My previous version advocated following the finalizer pattern in all cases, which was (correctly) pointed out to be against the framework design guidelines. However, in the event that you're actually dealing with unmanaged resources (for example, you're making direct P/Invoke calls and obtaining a handle that needs to be explicitly freed) it's advisable that you create a finalizer and call Dispose within it to protect against people who consume your code and don't call Dispose:

public class YourClass : IDisposable
{
    private OtherDisposableType yourResource;

    public YourClass()
    {
        yourResource = new OtherDisposableType();
    }

    public void Dispose()
    {
        yourResource.Dispose();

        GC.SuppressFinalize(this);
    }

    ~YourClass()
    {
        Dispose();
    }
}
Adam Robinson
Great thanks, but won't the destructor be called when the instance is eligible for GC, and I'd like my static variable to persist across instances?
Ben Aston
+1 to (a) counteract the cowardly -1 without a comment and (b) because I believe implementing IDisposable on your class is the correct solution here.
TrueWill
@Ben: Static variables are not what I'd recommend. If you feel strongly that you need to go that route, then the only way would be to add a static `Dispose` method on your class that you explicitly call when your application shuts down (or at whatever appropriate time there may be).
Adam Robinson
It started out good. But went downhill after recommending to implement a finalizer. Delete that and you'll have posted a good answer.
Hans Passant
Actually Microsoft recommends against creating a finalizer unless you're dealing with unmanaged resources. Even then, you can generally avoid it by using wrappers like SafeHandle. See the section on the Dispose Pattern in the book Framework Design Guidelines for details. http://www.amazon.com/Framework-Design-Guidelines-Conventions-Libraries/dp/0321545613
TrueWill
@Adam - ok, thanks for the clarification. I'll avoid the static variable this time, but I wanted a full understanding of the hypothetical situation of having a static variable.
Ben Aston
You should never invoke other objects from a finalizer - they may already have been finalized themselves! If the object is wrapping some unmanaged resource that must absolutely be released, then it will already have its own finalizer.
Wim Coenen
A reference for my comment: "The Finalize method should not reference any other objects." http://msdn.microsoft.com/en-us/library/b1yfkh5e%28VS.71%29.aspx
Wim Coenen
+1  A: 

So basically you want to cache an expensive resource. That's a good thing.

Global data (static variables in this case) is not such a good thing, IMHO. Instead, why not make it an instance variable and control the lifetime?

Write your class that handles the AD responsibilities, have it create and use the PrincipalContext, and make it IDisposable as well (using the Dispose Pattern). Extract an interface from it to decouple it and make classes that use it easier to test.

Classes that want to use AD services will take a constructor parameter of your new interface (Dependency Injection or DI). You can either create your class manually in a using block and pass it to the classes or use a DI Container Framework. You can have the framework set the lifetime of the AD object to the lifetime of the container (which may also be IDisposable). See How do you reconcile IDisposable and IoC? and your DI Container documentation for more on this.

TrueWill
I really like this answer, thanks. The discussion generated has left the academic question in my mind as to how best deal with the disposal of IDisposable types stored in class variables (this may be due to deficiencies in my understanding however).
Ben Aston
@Ben - Thanks! I agree that we haven't answered that question (although Adam Robinson's comment on a static Dispose method is worth considering). I tend to view static/class variables (except for readonly immutables like strings and Regex's) as code smells, similar to global variables in other languages.
TrueWill
+2  A: 

Look at what the System.ComponentModel namespace does. Basically, the pattern I typically use is to have a collection of subcomponents, which includes everything I own that's not a 'value' - whether or not it implements IDisposable.

Then, when I Dispose() myself, I iterate over this collection and Dispose anything that implements IDisposable.

One advantage of this technique is that if an object I own starts out not being Disposable, but later adds the IDisposable interface, my class will do the right thing without ever having to change.

Also, use of a DI/IoC container can handle much of this for you.

kyoryu
An interesting discussion of this: http://blogs.msdn.com/kcwalina/archive/2006/02/10/Disposable-Collection.aspx
TrueWill
Not quite what I was talking about - not necessarily a disposable collection, but rather any subcomponents of an object being stored in a collection (of whatever type). The idea of a disposable collection sounds a bit scary to me, though I can't quite articulate why at the moment.
kyoryu