5

I have the code below to query records from a stored procedure but am concerned I may not be disposing what I need to or am disposing when the object would be cleared by the Garbage Collector shortly afterward anyway.

Do I need to dispose the SqlDataReader since it is within the try catch block?

Do I need to run both cmd.Dispose and cmd.Connection.Close or does one infer the other?

Would the garbage collector eventually dispose of all these objects anyway (maybe not timely enough) or do these objects implictly require dispose possibly because of using unmanaged code?

public void GetData(string studentID)
    {
        SqlCommand cmd = new SqlCommand("sp_stored_proc", 
                new SqlConnection(Settings.Default.connectionString)) 
                { CommandType = CommandType.StoredProcedure };
        try
        {
            cmd.Connection.Open();
            cmd.Parameters.AddWithValue("@student_id", studentID);
            SqlDataReader dr = cmd.ExecuteReader();

         //do something with the data

            if (dr != null)
                dr.Dispose();
        }
        catch
        {
            //error handling
        }
        finally
        {
            if (cmd != null)
            {
                cmd.Dispose();
                cmd.Connection.Close();
            }

        }

    }
PeteT
  • 18,754
  • 26
  • 95
  • 132

6 Answers6

18

You should dispose the data reader, and the command. No need to separately close the connection if you dispose the command. You should ideally do both using a using block:

using (SqlCommand cmd = new...)
{
    // do stuff
    using (SqlDataReader dr = cmd.ExecuteReader())
    {
        // do stuff
    }
}

If you need exception handling do that separately either inside or around the using blocks - no need for the finally for the Dispose calls though with using.

David M
  • 71,481
  • 13
  • 158
  • 186
  • 1
    If I could upvote this more than once, I would. COME ON PEOPLE! USE USING BLOCKS! – Andy_Vulhop Jul 21 '09 at 13:55
  • 2
    Disposing the `SqlCommand` object will **not** dispose the `SqlConnection`. This is very easy to test. http://stackoverflow.com/questions/60919/is-sqlcommand-dispose-enough/60934#60934 – arcain Sep 06 '11 at 15:19
  • Must I dispose the reader even if I dispose the command? Must I dispose the command even if I dispose the connection? Can you refer us to any source of this? – Lii Mar 24 '15 at 08:17
3

Do I need to dispose the SqlDataReader since it is within the try catch block?

-- Yes, as being inside of the try catch will not call the dispose method.

Do I need to run both cmd.Dispose and cmd.Connection.Close or does one infer the other?

-- Yes, you need to run both. Calling Cmd.dispose does not close the connection.

The dispose method is meant to be used by the programmer to clean up resources which either aren't directly managed by the garbage collector, or the need to be cleared out after the program is done using them to free up space. Technically, one could set up the program so the GC would handle it's disposal, but that's an assumption I wouldn't make, especially since the programmer writing the class exposed the dispose method for you. Putting the Command in a using statement is probably the easiest route, because you know it will get disposed when the code leaves the declaration space.

using (var connection  = new Connection ())
{
   using (var cmd = new Command())
   {



   }
}
arcain
  • 14,920
  • 6
  • 55
  • 75
kemiller2002
  • 113,795
  • 27
  • 197
  • 251
  • "no, cmd.dispose will close the connection" - I think that is not correct; as far as I know, calling Dispose on the command has nothing to do with its connection. – Fredrik Mörk Jul 21 '09 at 12:05
  • @Kevin: the post you linked to states that calling Dispose on a *connection object* will call close on that same object. The word "command" does not appear on the page. – Fredrik Mörk Jul 21 '09 at 12:15
  • I don't believe I made that mistake. After I thought about that for a second, it makes total sense a command object wouldn't close a connection object. Doh! – kemiller2002 Jul 21 '09 at 12:45
3

If you use something like this:

public void GetData(string studentID)
{
    using (SqlConnection connection = new SqlConnection(Settings.Default.connectionString))
    {
        connection.Open();

        using (SqlCommand command = connection.CreateCommand())
        {
            command.CommandType = CommandType.StoredProcedure;
            command.CommandText = "sp_stored_proc";
            command.Parameters.AddWithValue("@student_id", studentID);

            using (SqlDataReader dataReader = command.ExecuteReader())
            {
                // do something with the data
            }
        }
    }
}

then all of your Disposable objects will get disposed of correctly. Calling Dispose() on the SqlConnection, SqlCommand and SqlDataReader objects (which is what the using block does when it exits) closes them correctly.

Additionally, this approach keeps all of your variables scoped to where they are used.

The downside to this approach is that if you need error handling using a try/catch, you have to either wrap it around the whole method body, or use several of them to handle connection errors differently from reading errors, etc...

adrianbanks
  • 81,306
  • 22
  • 176
  • 206
2

Personally if something has a dispose method then it is worth using it anyway as they will prevent protential memory leaks.

AutomatedTester
  • 22,188
  • 7
  • 49
  • 62
1

To make a long story short; if it implements IDisposable, you should call Dispose.

Even if you use Reflector to figure out that Dispose in one object invokes Dispose on another object, I would still recommend to call Dispose on both, since this is internal implementation details that may change in some future release, so you should not rely on that always being true.

So, Dispose anything that is IDisposable.

Fredrik Mörk
  • 155,851
  • 29
  • 291
  • 343
-4

you should open Connection firstly by Connection.Open(); then use the methods such as SqlDataReader to read after all,close SqlDataReader firstly and then close connection

you can use keyword "using" to dispose it, but it is not a good idea

in fact the keyword "using" is to dispose the object automatically. in other word,the object should achieve dispose method

Edwin Tai
  • 1,224
  • 1
  • 13
  • 19