CODEWITHSUNDEEP
c#.netsql-serverado.net
Dennis
Dennis

Reputation: 14475

SqlConnection SqlCommand SqlDataReader IDisposable

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!

Upvotes: 20

Views: 10325

Answers (7)

Geeky Guy
Geeky Guy

Reputation: 9399

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.

Upvotes: 2

Matthias Meid
Matthias Meid

Reputation: 12523

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.

Upvotes: 5

SamiHuutoniemi
SamiHuutoniemi

Reputation: 1595

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.

Upvotes: -1

Eren Ersönmez
Eren Ersönmez

Reputation: 39085

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.

Upvotes: 4

Tony Hopkinson
Tony Hopkinson

Reputation: 20320

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.

Upvotes: 1

Mike Perrenoud
Mike Perrenoud

Reputation: 67898

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.

Upvotes: 16

Davide Lettieri
Davide Lettieri

Reputation: 326

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
 }

Upvotes: -2

Related Questions