Do I need to close and dispose the SQLConnection explicitly?

Question

SqlDataReader rdr = null; con = new SqlConnection(objUtilityDAL.ConnectionString); using (SqlCommand cmd = con.CreateCommand()) { try { if (con.State != ConnectionState.Open) con.Open(); cmd.CommandType = CommandType.StoredProcedure; cmd.Parameters.Add(Parameter); cmd.CommandText = _query; rdr = cmd.ExecuteReader(); } catch (Exception ex) { throw ex; } }

In this above code, sqlconnection is opened inside the managed code. Hence, will the connection object be disposed automatically upon ending the scope of USING?

Your connection object is not inside the scope of a using statement. — Steve
– Steve, Commented Apr 17, 2018 at 6:57

Christos · Accepted Answer · 2018-04-17 07:30:16Z

7

You have to care about the disposal of SqlConnection, since it's a disposable object. You can try this:

using(var sqlConnection = new SqlConnection(objUtilityDAL.ConnectionString)) { using (SqlCommand cmd = con.CreateCommand()) { // the rest of your code - just replace con with sqlConnection } }

I suggest you replace the con with a local variable - if there isn't already (it does not evident from the code you have posted). There is no need for using a class field for this purpose. Just create a local variable, it would be far more clear to track it.

edited Apr 17, 2018 at 7:30

answered Apr 17, 2018 at 6:51

Christos

54.1k9 gold badges82 silver badges114 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

L. Guthardt Over a year ago

It looks like that con might be a field. I don't suggest using a field in the using statement. Because once it got disposed in this method and you try to use the field somewhere else you will get an exeption, because the object got disposed. Wouldn't be using a local variable better to avoid above situation? Moreover a field which gets disposed doesn't even have any advantage versus a local variable. Or am I wrong?

Christos Over a year ago

@L.Guthardt By reading the post a bit fast, I did't notice that this might be a class field :). Well said ! As I have written in my update, I don't think that's a good idea to make use of class field for that purpose.

L. Guthardt Over a year ago

Thank you. :) Very nice that you updated your answer to add this information. +1

Chinchan Over a year ago

Thanks so much Christos. Your point suggesting me to use local variable instead of class variable makes sense as it will get disposed inside the method. I would change the logic. Thanks again!!

Dmitrii Bychenko · Accepted Answer · 2019-11-01 06:40:22Z

You should Dispose every temporary IDisposable instance you create manually, i.e. wrap them into using:

 // Connecton is IDisposable; we create it // 1. manually - new ... // 2. for temporary usage (just for the query) using (SqlConnection con = new SqlConnection(objUtilityDAL.ConnectionString)) { // Check is redundant here: new instance will be closed con.Open(); // Command is IDisposable using (SqlCommand cmd = con.CreateCommand()) { cmd.CommandType = CommandType.StoredProcedure; cmd.Parameters.Add(Parameter); cmd.CommandText = _query; // Finally, Reader - yes - is IDisposable using (SqlDataReader rdr = cmd.ExecuteReader()) { // while (rdr.Read()) {...} } } }

Please, notice that

 try { ... } catch (Exception ex) { throw ex; }

is at least redundant (it does nothing but rethrow the exception, while loosing stack trace) and that's why can be dropped out

Do NOT rethrow exception as it, or you'll lose the stack trace information... Use only throw;

Zohar Peled · Accepted Answer · 2018-04-17 08:46:08Z

In general:

In the .Net framework, nothing gets disposed automatically. Otherwise we wouldn't need the IDisposable interface. The garbage collector can only handle managed resources, so every unmanaged resource must be handled in code in the dispose method.

So as a rule, every instance of every class that is implementing the IDisposable must be disposed either explicitly by calling it's Dispose method or implicitly with the using statement.

As best practice, you should strive to use anything that implements the IDisposable interface as a local variable inside a using statment:

using(var whatever = new SomeIDisposableImplementation()) { // use the whatever variable here }

The using statement is syntactic sugar. the compiler translates it to something like this:

var whatever = new SomeIDisposableImplementation(); try { // use the whatever variable here } finally { ((IDisposable)whatever).Dispose(); }

Since the finally block as guaranteed to run regardless of whatever happens in the try block, your IDisposable instance is guaranteed to be properly disposed.

With SqlConnection specifically, in order to return the connection object back to the connection pool, you must dispose it when done (the Dispose method will also close the connection, so you don't need to explicitly close it) - So the correct use of SqlConnection is always as a local variable inside the using statement:

using(var con = new SqlConnection(connectionString)) { // do stuff with con here }

Collectives™ on Stack Overflow

Do I need to close and dispose the SQLConnection explicitly?

3 Answers 3

4 Comments

2 Comments

2 Comments

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

2 Comments

2 Comments

Linked

Related