Using C#'s 'using' statement with a custom object's function, do I Need to implement IDisposable?

Question

I have an sqlConnection manager class like so:

public class SQLConn {
  public string connStr = System.Configuration.ConfigurationSettings.AppSettings["ConnectionString"];

  private SqlConnection sqlConn;

  public SqlConnection Connection()
  {
      sqlConn = new SqlConnection(connStr);

      return sqlConn;
  }

  public void Open()
  {
        sqlConn .Open();
  }
}

If I use a function with the 'using' statement like:

var conn = new SQLConn();

using (conn.Connection()) 
{ 
    String query = "Select * from table";
    objSql = new SqlCommand(query, conn.Connection());      

    conn.Open(); 
    DoSomething(); 
}

Does the using statement dispose of the connection automatically since conn.Connection() returns a SqlConnection object? Or, do I have to implement IDisposable and a custom Dispose method on the SqlConn class?

Is this even a good way at all? I'm working with legacy code and I'm not able to use an ORM yet but is there a way to simplify this existing pattern to manage/create SQL connections?

Solution 1

The using statement will look at the final type of the expression - i.e. whatever is returned from .Connection(); if this returns something that is IDisposable, then you're OK.

The compiler will tell you if you get it wrong ;-p (it won't let you use using on something that isn't IDisposable).

You should probably watch out for where you are creating two connections:

using (var c = conn.Connection()) // <==edit
{ 
    String query = "Select * from table";
    objSql = new SqlCommand(query, c); // <==edit

    c.Open(); 
    DoSomething(); 
}

and possibly:

public SqlConnection Connection()
{
  if(sqlConn == null) sqlConn = new SqlConnection(connStr); // <== edit
  return sqlConn;
}

Solution 2

It will work but after the using {} you will be left with an sqlConn that internally holds a Disposed SqlConnection. Not a really useful situation

Solution 3

Your code is wrong!

shoudl be something like this:

Dim conn as New SQLConn();
Dim sqlConnection New SQLConnection();

sqlConnection = conn.Connection();

using (sqlConnection) 
{ 
    String query = "Select * from table";
    objSql = new SqlCommand(query, sqlConnection);      

    conn.Open(); 
    DoSomething(); 
}

That way the using statement will dispose the connection at the end.

Solution 4

To clarify what is being said above:

Any object you need to use with using should be disposed at the end of the using statement. The compiler thus need to make sure that your type implements the IDisposable interface when it sees using on that type object or it won't let you go.

Solution 5

to answer your headline question, you must implement IDisposable in the class whose object you're using with "using". Otherwise, you'll get a compile-time error.

Then, yes, "using" will dispose your SqlConnection at the end of the block. Think of "using" as a "try-finally": there is an implicit call to Dispose() in the "finally" block.

Finally, cleaner code would be:

using( SqlConnection = new SqlConnection( connStr ) {
    // do something
}

At least readers of your code won't have to make the mental effort to realize as Henk Holterman pointed out that your SQLConn object holds a reference to a disposed connection.

View more solutions

Comments

[Marc Gravell]

The final type of the expression does need to implement IDisposable: Error 1 'Bar': type used in a using statement must be implicitly convertible to 'System.IDisposable

[Ruben]

Why do you create two SqlConnections? At least one will not be disposed

[Noam Gal]

+1 The problem is not with writing using (conn.Connection()), but by the fact that the Connection() method creates a new object every time it is being called, and in the original post, the connection which is under the "using" is not a different instance than the connection actually being used inside the block.

[Marc Gravell]

You probably don't need the "New SQLConnection", then

[leppie]

Hmmm, interesting, is that changed behaviour from previous versions of the compiler, or have I just miss understood it all along?

[Marc Gravell]

I'm not aware of a change (but I haven't looked at the 1.2 spec lately). Are you perhaps thinking of foreach? where the enumerator might be disposable (and if so, is disposed), but doesn't have to be? Or perhaps the fact that the returned value is allowed to be null?

[Marc Gravell]

In fact, I doubt it changed - since that would clearly be a breaking change that stopped some existing code from compiling; the C# team try very hard to avoid that...

[leppie]

You know I am probably thinking of foreach :) Damned reflector hiding all the details makes you forget!

[Pavel Nikolov]

Yes we don't need new SqlConnection - sorry for that (I use C# only and don't understand VB at all) @Noam Gal - My point was that the provided code is useless, because the SQLConn class doesn't return a single SqlConnection instance. So in the code provided in the question we have one connection (which is disposed by the using statement) and another which is used by the code and is not disposed at the end...