IDisposable or not in custom classes

[Mark Rae]

Hi,

Following on from the recent thread about why HttpWebRequest doesn't
implement the IDisposable interface, I got to wondering whether people make
their custom classes inherit IDisposable or not as a general rule, or only
under certain circumstances...

Since it's an easy enough thing to do
(http://msdn2.microsoft.com/en-us/library/system.idisposable.aspx), is there
any good reason not to make every custom class IDisposable, the same way as
there's no good reason not to use exception handling in every method...?

I can understand the rationale for implementing IDisposable for classes
which e.g. use unmanaged resources, or which open files and/or streams,
because the GC won't know about them, but what about for classes which use /
reference only managed code? Would it be overkill to make those classes
IDisposable, and would it simply be enough to destroy references to them by
variables which have instantiated them to null...?

Interested to know your opinions...

Mark

 

[Dave Sexton]

Hi Mark,

Following on from the recent thread about why HttpWebRequest doesn't
implement the IDisposable interface, I got to wondering whether people
make their custom classes inherit IDisposable or not as a general rule, or
only under certain circumstances...

IDisposable should be implemented if your class composites:

A. unmanaged resources
B. IDisposable types

If your class doesn't manage the lifetime of unmanaged or IDisposable
references then IDisposable isn't necessary. In all other circumstances,
IDisposable isn't necessary either.

Since it's an easy enough thing to do
(http://msdn2.microsoft.com/en-us/library/system.idisposable.aspx), is
there any good reason not to make every custom class IDisposable, the same
way as there's no good reason not to use exception handling in every
method...?

I strongly disagree that exception handling should be placed in every
method:

http://davesexton.com/cs/blogs/blog/archive/2006/12/05/Handling-Exceptions-for-Contingency.aspx

I can understand the rationale for implementing IDisposable for classes
which e.g. use unmanaged resources, or which open files and/or streams,
because the GC won't know about them, but what about for classes which use
/ reference only managed code? Would it be overkill to make those classes
IDisposable, and would it simply be enough to destroy references to them
by variables which have instantiated them to null...?

Overkill

 

[Dustin Campbell]

I can understand the rationale for implementing IDisposable for

classes which e.g. use unmanaged resources, or which open files and/or
streams, because the GC won't know about them, but what about for
classes which use / reference only managed code? Would it be overkill
to make those classes IDisposable, and would it simply be enough to
destroy references to them by variables which have instantiated them
to null...?

This is most certaily overkill. IMO, it betrays a mistrust of the garbage
collector.

Best Regards,
Dustin Campbell
Developer Express Inc

 

[Brian Gideon]

Mark,

You've received some good comments already. Let me go a different
direction. I think there are times where implementing IDisposable
makes since even if the class does not hold unmanaged resources or
managed types that implement IDisposable. Candidates include anything
with Acquire/Release, Open/Close, Enter/Exit, etc. semantics.

Jon Skeet provides the best example of this. By wrapping the Monitor
in a IDisposable object you can take advantage of the using construct
instead of lock. The wrapping class would not comply with the
canonical rules for IDisposable implementation. Nevertheless, I think
it is acceptable.

http://www.yoda.arachsys.com/csharp/miscutil/usage/locking.html

P.S. Too bad Microsoft didn't take full advantage of "using". It could
have prevented the addition of another keyword (namely "lock").

Brian

 

[Mark Rae]

Dave,

IDisposable should be implemented if your class composites:

A. unmanaged resources
B. IDisposable types

If your class doesn't manage the lifetime of unmanaged or IDisposable
references then IDisposable isn't necessary. In all other circumstances,
IDisposable isn't necessary either.

OK - that makes sense.

Related to this, am I right in thinking that if you instantiate a custom
class which implements IDisposable with the using (...) syntax, the class
*needs* to have a .Dispose() method? I.e. when the code encapsulated by
using (...) exists, does it look for a Dispose() method in the class which
it instantiated, or does it dispose of it in some other way...?

Mark

 

[Nicholas Paldino [.NET/C# MVP]]

Mark,

Can you show the thread that you are referring to? I don't know if it
was pointed out, but there is no reason to implement IDisposable on the
request since the request doesn't do anything until it is sent. The
WebRequest model basically places the managability of the unmanaged stuff in
the Response (which you are going to get most likely because you made the
request in the first place) and then call Dispose of it there.

And yes, it is overkill to implement it on all the classes. There are
very specific guidelines in place for when to implement IDisposable.

Hope this helps.

 

[Mark Rae]

If you have the class implementing the IDisposable interface then it must
have a Dispose() method since that is part of the interface.

D'oh! I really could have found that out for myself...Apologies.

I'm learning a lot of good practices here, though...

OK, here's another one...

Let's say you have a Database Abstraction Layer class called MyDAL which,
for the sake of argument, has a static method called GetSqlDataReader which
accepts a string of SQL as its only argument and returns an SqlDataReader
object). Barring the obvious comments about using stored procedures to avoid
SQL Injection, it might look something like this:

public static SqlDataReader GetSqlDataReader(string strSQL)
{
try
{
using (SqlConnection objSqlConn = new SqlConnection(<read connection
string from config file>))
{
using (SqlCommand objSqlCommand = new SqlCommand(strSQL,
objSqlConn))
{
objSqlCommand.Connection.Open();
return
(objSqlCommand.ExecuteReader(CommandBehavior.CloseConnection));
}
}
}
catch (SqlException ex)
{
throw ex;
}
catch(Exception)
{
throw;
}
}

You might use it something like this:

using (SqlDataReader objDR = MyDAL.GetSqlDataReader("SELECT * FROM
MyTable"))
{
while (objDR.Read())
{
// do something
}
objDR.Close(); // is this necessary?
}

Since SqlDataReader is disposable, and it has been instantiated with the
using (...) syntax, and the reader itself has been created with the
ExecuteReader method of a SqlCommand object specifying the
CommandBehavior.CloseConnection property), is the line "objDR.Close()"
necessary...?

 

[Mark Rae]

message
Nicholas,

Can you show the thread that you are referring to?

Its subject is "HttpWebRequest and IDisposable" and it was started by me in
this group on 10th December.

I don't know if it was pointed out, but there is no reason to implement
IDisposable on the request since the request doesn't do anything until it
is sent.

It's actually not possible to implement it because it's not disposable, much
to everyone's surprise...

And yes, it is overkill to implement it on all the classes.
Indeed.

There are very specific guidelines in place for when to implement
IDisposable.

Are there? Are they available for download anywhere...?

Mark

 

[Samuel R. Neff]

Close() is not necessary when using "using".

Also note that since your class is returning a connected Reader
instance you do not want to use "using" on the SqlConnection object
'cause you don't want to close the connection. That's why readers
have the CloseConnection behavior, you don't have to worry about
closing the connection 'cause the reader closes it automatically when
the reader itself is closed.

HTH,

Sam

 

[Mark Rae]

Close() is not necessary when using "using".
OK.

Also note that since your class is returning a connected Reader
instance you do not want to use "using" on the SqlConnection object
'cause you don't want to close the connection. That's why readers
have the CloseConnection behavior, you don't have to worry about
closing the connection 'cause the reader closes it automatically when
the reader itself is closed.

Thanks very much.

 

[Dave Sexton]

Hi Mark,

Are there? Are they available for download anywhere...?

".NET Framework Developer's Guide, Implementing Finalize and Dispose to
Clean Up Unmanaged Resources"
http://msdn2.microsoft.com/en-us/library/b1yfkh5e.aspx

".NET Framework Developer's Guide, Implementing a Dispose Method"
http://msdn2.microsoft.com/en-us/library/fs2xkftw.aspx

Check out the TOC on the left and you'll see a whole bunch of topics that
provide development guidelines for many other things.

Note that the guidelines recommending to use Dispose along with a finalizer
can be accomplished easily by deriving your class from Component, which
provides the recommended behavior already; however, that certainly doesn't
mean you should derive all of your classes from Component just like you
shouldn't implement IDisposable on all of your classes either. Deriving
from Component also provides base designer support for your business objects
so they can be added to the component tray from the toolbox, a protected
EventHandlerList used by derived Controls, and a Container/Site pattern
implementation for designers, which may be useful to you in and of itself.

Realize too that you don't need to have a finalizer on your classes (that
don't derive from Component) if they simply aggregate or composite
references to IDisposables since each IDisposable reference should have
already implemented their own finalization code to release the unmanaged
resources.

 

[Mark Rae]

".NET Framework Developer's Guide, Implementing Finalize and Dispose to
Clean Up Unmanaged Resources"
http://msdn2.microsoft.com/en-us/library/b1yfkh5e.aspx

".NET Framework Developer's Guide, Implementing a Dispose Method"
http://msdn2.microsoft.com/en-us/library/fs2xkftw.aspx

Check out the TOC on the left and you'll see a whole bunch of topics that
provide development guidelines for many other things.

Got a bit of reading to do, then...

Thanks very much.