SqlConnection SqlCommand SqlDataReader IDisposable

Question

SqlConnection, SqlCommand and SqlDataReader all implement the IDisposable interface. I read about a best practice to always wrap IDisposables into a using block.

So, my common scenario for querying data would look something like this (in greater context of course a mapping tool like linq2sql would be suitable, but lets just assume we want to use this approach here):

using (SqlConnection cn = new SqlConnection("myConnectionstring"))
{
    using (SqlCommand cm = new SqlCommand("myQuery", cn))
    {
        // maybe add sql parameters
        using (SqlDataReader reader = cm.ExecuteReader())
        {
             // read values from reader object
             return myReadValues;
        }
    }
}

Is this the correct way or could that be considered overkill? I am a little unsure about this level of nesting using blocks, but of course i want to do it the correct way. Thanks!

Answer 1

This is 100% the correct way. If a class leverages IDisposable it should be wrapped in a using statement to ensure that the Dispose() method is called. Further, communicating with an outside technology -unmanaged at that -like SQL Server shouldn't be taken lightly. The SqlCommand object implements IDisposable for a very good reason. The code below is the Dispose() method for the SqlCommand object:

protected override void Dispose(bool disposing)
{
    if (disposing)
    {
        this._cachedMetaData = null;
    }
    base.Dispose(disposing);
}

and as you can see, it's releasing a reference to the _cachedMetaData object so that it too can get cleaned up.

Answer 2

You can use the following way of typography to get the code closer to the left:

using (SqlConnection cn = new SqlConnection("myConnectionstring"))
using (SqlCommand cm = new SqlCommand("myQuery", cn))
using (SqlDataReader reader = cm.ExecuteReader())
{
     // read values from reader object
     return myReadValues;
}

As other already pointed out, using three nested using blocks is correct though.

Answer 3

That is the correct way, if you will be done with the reader. Sometimes, you need the reader to remain open (maybe your method returns it), so it disposing the reader right away would not work. In those cases, there is an overload of the ExecuteReader that can help you:

var cn = new SqlConnection("myConnectionstring");
var cm = new SqlCommand("myQuery", cn);
var reader = cm.ExecuteReader(CommandBehavior.CloseConnection);
return reader;

This will keep the connection and the reader open. Once the reader is closed/disposed, it will also close (and dispose) the connection as well.

using(var reader = GetReader()) //which includes the code above
{
   ...
} // reader is disposed, and so is the connection.

Answer 4

This is not overkill. A using block is good practice because it ensures that the Dispose() method of the object will be called, even if an exception is thrown.

There is a name for this kind of thing, though. It's called code sugar. So:

using (foo bar = new foo()) { //...snip }

Is short hand code for:

foo bar = null;
Exception error = null;
try {
    bar = new foo();
    // ...snip
}
catch (Exception ex) {
    error = ex;
}
finally {
    if (bar != null) bar.Dispose();
    if (error != null) throw error;
}

Either form is equal to the other, they're just different ways to write the same thing. In other words, same difference between for and while: they do basically the same thing but are used in different ways.

using is preferred because it makes the code shorter and more readable, and automates the disposal for you. As to whether you should use it, though, don't listen to people who say you should always do something. It's good practice, granted, but knowing when to use, why to use and the benefits and consequences of using, or not using something is worth way more than doing something because people say you should.

Eren's answer has an example of a case where you wouldn't want to have a using block for the reader.

Answer 5

Yep that's correct. You can miss the braces out in nested using as they are one statement, but I don't feel that adds to readability.

Answer 6

I don't see any point in disposing the SqlCommand. The SqlConnection and SqlDataReader should be disposed of though.

The SqlConnection because it won't close by itself when it goes out of scope.

The SqlDataReader can keep the SqlConnection busy until the reader is closed.

Not even MSDN examples dispose of the SqlCommand.

Answer 7

I'm not an expert but i know that the using is translated into a try/finally block maybe you can wrap the SqlConnection, SqlCommand and SqlDataReader into a unique try/finally

try {
     // code here with SqlConnection, SqlCommand and SqlDataReader
}
finally
{
  // Dispose call on SqlConnection, SqlCommand and SqlDataReader
 }