tags:

views:

272

answers:

5

Hi,

I've seen so much C# code in my time as a developer that attempt to help the GC along by setting variables to null or calling Dispose() on classes (DataSet for example) within thier own classes Dispose() method that I've been wondering if there's any need to implement it in a managed environment.

Is this code a waste of time in its design pattern?

class MyClass : IDisposable {

        #region IDisposable Members

        public void Dispose() {
otherVariable = null;
            if (dataSet != null)
                dataSet.Dispose();


        }

        #endregion
    }
+1  A: 

Not entirely. If you have member variables which are disposable, then you probably should dispose of it like that. Your object may live longer than the scope of the work it is doing as the garbage collector is not guaranteed to run at any particular time.

Setting managed variables to null is a waste of time though. The object won't get GC'd any faster.

Jamie Penney
+3  A: 

The GC does not call .Dispose() (It does, however, call the finalize ~MyClass() method, which you can provide a call to the Dispose() method to have resources automatically managed when the GC decides to clean up your class).

You must always provide a way of disposing internal resources such as DataSets to code that uses your classes (and ensure you actually call .Dispose() or wrap the constructor in a using). Using IDisposable on your classes that use internal resources is highly recommended.

From MSDN:

The primary use of this interface is to release unmanaged resources. The garbage collector automatically releases the memory allocated to a managed object when that object is no longer used. However, it is not possible to predict when garbage collection will occur. Furthermore, the garbage collector has no knowledge of unmanaged resources such as window handles, or open files and streams.

public void Dispose()
{
    otherVariable = null;
    if (dataSet != null)
    {
        dataSet.Dispose();
        dataSet = null;
    }
}
Codesleuth
+3  A: 

Read Jon Skeet's answer here; http://stackoverflow.com/questions/574019/calling-null-on-a-class-vs-dispose/574659#574659

GordonB
+2  A: 

No, Dispose methods are not a waste of time.

The dispose pattern is there to allow a caller to clean up a class as soon as they have finished with it, rather than waiting for the GC to collect it. The delay doesn't matter much for plain heap memory, which is why basic classes like String don't implement it. What Dispose is useful for however is cleaning up unmanaged resources. Somewhere internally, the Dataset class is using an unmanaged resource, so it provides a dispose method to allow you to let it know when that unmanaged resource can be released.

If the pattern has been followed correctly Dataset will also have a finalizer (or some subclass will) which means that if you didn't dispose of it manually, eventually the GC would run, the finalizer would get called and the unmanaged resource would be cleaned up that way. This unmanaged resource might be important though, imagine if it was a file lock, or a database connection, you don't really want to hang around waiting for the GC to run before you can reuse your database connection. Dispose provides a deterministic way of cleaning up resources when they are finished rather than relying on the non-deterministic GC.

As for setting variables to null in a dispose method. It nearly all cases it would be pointless. setting a variable to null removes a reference to that variable, which will make it eligible for garbage collection (if that's the last reference), but as you are disposing of the class anyway, you are likely to be going out of scope for the containing class so the internal class will become eligible for collection anyway.

If you have member variables inside your class that are disposable that you created (not just references you hold), then you should always call dispose on them from your own class's dispose method, but don't bother setting them to null.

Simon P Stevens
A: 

You can see my answer here

thecoop