views:

6739

answers:

14

I am just looking at the using statement, I have always known what it does but until now not tried using it, I have come up with the below code:

 using (SqlCommand cmd = 
     new SqlCommand(reportDataSource, 
         new SqlConnection(Settings.Default.qlsdat_extensionsConnectionString)))
 {
     cmd.CommandType = CommandType.StoredProcedure;
     cmd.Parameters.Add("@Year", SqlDbType.Char, 4).Value = year;
     cmd.Parameters.Add("@startDate", SqlDbType.DateTime).Value = start;
     cmd.Parameters.Add("@endDate", SqlDbType.DateTime).Value = end;
     cmd.Connection.Open();

     DataSet dset = new DataSet();
     new SqlDataAdapter(cmd).Fill(dset);
     this.gridDataSource.DataSource = dset.Tables[0];
 }

This seems to work but is there any point in this since as far as I can tell I would still need to enclose this in a try catch block to catch unforseen errors e.g. sql server down. Am I missing something?

As far as I can currently see it just stops me closing and disposing of cmd but there will be more lines of code due to the try catch still being needed.

+2  A: 

using isn't about catching exceptions. It's about properly disposing of resources that are outside the view of the garbage collector.

David B
I see what your saying but I can't see an advantage over a try catch finally block with close and dispose statements.
PeteT
Resources may not be outside the view of the garbage collector. It is still helpful to clean them up ASAP rather than wait for GC.
Jennifer Zouak
@David: it isn't about the garbage collector, either.
John Saunders
I didn't say it was.
David B
+1  A: 

Yes you would still need to catch exceptions. The benefit of the using block is you are adding scope to your code. You are saying, "Within this block of code do some stuff and when it gets to the end, close and dispose of resources"

It's not completely necessary at all, but it does define your intentions to anyone else using your code, and it also helps not leaving connections etc open by mistake.

KiwiBastard
+7  A: 

This code should be as follows to ensure timely closing of the connection. Closing just the command doesn't close the connection:

using (SqlConnection con = new SqlConnection(Settings.Default.qlsdat_extensionsConnectionString))
using (SqlCommand cmd = new SqlCommand(reportDataSource, con))
         {
             cmd.CommandType = CommandType.StoredProcedure;
             cmd.Parameters.Add("@Year", SqlDbType.Char, 4).Value = year;
             cmd.Parameters.Add("@startDate", SqlDbType.DateTime).Value = start;
             cmd.Parameters.Add("@endDate", SqlDbType.DateTime).Value = end;
             cmd.Connection.Open();

             DataSet dset = new DataSet();
             new SqlDataAdapter(cmd).Fill(dset);
             this.gridDataSource.DataSource = dset.Tables[0];
         }

To answer your question, you can do the same in a finally block, but this scopes the code nicely and ensures that you remember to clean up.

TheSoftwareJedi
+8  A: 

When doing IO work I code to expect an exception.

SqlConnection conn = null;
SqlCommand cmd = null;

try
{
    conn = new SqlConnection(Settings.Default.qlsdat_extensionsConnectionString)
    cmd = new SqlCommand(reportDataSource, conn);
    cmd.CommandType = CommandType.StoredProcedure;
    cmd.Parameters.Add("@Year", SqlDbType.Char, 4).Value = year;
    cmd.Parameters.Add("@startDate", SqlDbType.DateTime).Value = start;
    cmd.Parameters.Add("@endDate", SqlDbType.DateTime).Value = end;
        cmd.Open();

    DataSet dset = new DataSet();
    new SqlDataAdapter(cmd).Fill(dset);
    this.gridDataSource.DataSource = dset.Tables[0];
}
catch(Exception ex)
{
    Logger.Log(ex);
    throw;
}
finally
{
    if(conn != null)
        conn.Dispose();

        if(cmd != null)
        cmd.Dispose();
}

Edit: To be explicit, I avoid the using block here because I believe it to be important to log in situations like this. Experience has taught me that you never know what kind of weird exception might pop up. Logging in this situation might help you detect a deadlock, or find where a schema change is impacting a little used and little tested part of you code base, or any number of other problems.

Edit 2: One can argue that a using block could wrap a try/catch in this situation, and this is completely valid and functionally equivalent. This really boils down to preference. Do you want to avoid the extra nesting at the cost of handling your own disposal? Or do you incur the extra nesting to have auto-disposal. I feel that the former is cleaner so I do it that way. However, I don't rewrite the latter if I find it in the code base in which I am working.

Jason Jackson
Why the explicit disposal, when the using block does the exact same thing far more elegantly?
Justice
So I can log the exception. There is nothing elegant in a runtime exception on a user's desktop in a remote part of the country/world, and you have no clue what went wrong.
Jason Jackson
But the explicit disposal vs using 'using' has absolutely no effect on whether/how you log exceptions. You can easily use the 'using' block and, inside the 'using' block, have a try block in which the operation is performed and a catch block for logging exceptions.
Justice
I personally feel that a using block wrapping a try/catch is messy. The using is just doing the equivalent of a try/finally, so why not just write your own try/catch/finally instead and avoid some of the nesting? I realize this is a matter of personal preference, but I feel my preference here is just as valid as the one you have stated.
Jason Jackson
A: 

The using statement is actually changed into a try/finally block by the compiler in which the parameter of the using block is disposed of so long as it implements the IDisposable interface. Aside from ensuring the specified objects are properly disposed when they fall out of scope, there is really no error capturing gained by using this construct.

As is mentioned by TheSoftwareJedi above, you will want to make sure both the SqlConnection and SqlCommand objects are disposed of properly. Stacking both into a single using block is a bit messy, and might not do what you think it does.

Also, be mindful of using the try/catch block as logic. It's a code smell that my nose has a particular dislike for, and often used by newbies or those of us in a big hurry to meet a deadline.

Chris Ballance
+1  A: 

FYI, in this specific example, because you're using an ADO.net connection and Command object, be aware that the using statement just executes the Command.Dispose, and the Connection.Dispose() which do not actually close the connection, but simply releases it back into the ADO.net Connection pool to be reused by the next connection.open ... which is good, and the absolutely correct thing to do, bc if you don't, the connection will remain unuseable until the garbage collector releases it back to the pool, which might not be until numerous other connection requests, which would otherwise be forced to create new connections even though there's an unused one waiting to be garbage collected.

Charles Bretana
+2  A: 

Elaborating on what Chris Ballance said, the C# specification (ECMA-334 version 4) section 15.13 states "A using statement is translated into three parts: acquisition, usage, and disposal. Usage of the resource is implicitly enclosed in a try statement that includes a finally clause. This finally clause disposes of the resource. If a null resource is acquired, then no call to Dispose is made, and no exception is thrown."

The description is close to 2 pages - worth a read.

In my experience, SqlConnection/SqlCommand can generate errors in so many ways that you almost need to handle the exceptions thrown more than handle the expected behaviour. I'm not sure I'd want the using clause here, as I'd want to be able to handle the null resource case myself.

Kevin Haines
+1  A: 

There are a lot of great answers here, but I don't think this has been said yet.

No matter what... the "Dispose" method WILL be called on the object in the "using" block. If you put a return statement, or throw an error, the "Dispose" will be called.

Example:

I made a class called "MyDisposable", and it implements IDisposable and simply does a Console.Write. It always writes to the console even in all these scenarios:

using (MyDisposable blah = new MyDisposable())
{
    int.Parse("!"); // <- calls "Dispose" after the error.

    return; // <-- calls Dispose before returning.
}
Timothy Khouri
+2  A: 

If your code looks like this:

using (SqlCommand cmd = new SqlCommand(...))
{
  try
  {
    /* call stored procedure */
  }
  catch (SqlException ex)
  {
    /* handles the exception. does not rethrow the exception */
  }
}

Then I would refactor it to use try.. catch.. finally instead.

SqlCommand cmd = new SqlCommand(...)
try
{
  /* call stored procedure */
}
catch (SqlException ex)
{
  /* handles the exception and does not ignore it */
}
finally
{
   if (cmd!=null) cmd.Dispose();
}

In this scenario, I would be handling the exception so I have no choice but to add in that try..catch, I might as well put in the finally clause and save myself another nesting level. Note that I must be doing something in the catch block and not just ignoring the exception.

jop
A: 

If the caller of your function is responsible for dealing with any exceptions the using statement is a nice way of ensuring resources are cleaned up no matter the outcome.

It allows you to place exception handling code at layer/assembly boundaries and helps prevent other functions becoming too cluttered.

Of course, it really depends on the types of exceptions thrown by your code. Sometimes you should use try-catch-finally rather than a using statement. My habit is to always start with a using statement for IDisposables (or have classes that contain IDisposables also implement the interface) and add try-catch-finally as needed.

Andrew Kennan
+6  A: 

There may be no advantage to using a using statement in this case if you're going to have a try/catch/finally block anyway. As you know, the using statement is syntactic sugar for a try/finally that disposes of the IDisposable object. If you're going to have your own try/finally anyway, you can certainly do the Dispose yourself.

This really mainly boils down to style - your team may be more comfortable with using statements or using statements may make the code look cleaner.

But, if the boilerplate the using statement would be hiding is there anyway, go ahead and handle things yourself if that's your preference.

Michael Burr
A: 

So, basically, "using" is the exact same as "Try/catch/finally" only much more flexible for error handling.

Craig
A: 

Minor correction to the example: SqlDataAdapter also needs to be instantiated in a using statement:

using (SqlConnection con = new SqlConnection(Settings.Default.qlsdat_extensionsConnectionString))
{
    using (SqlCommand cmd = new SqlCommand(reportDataSource, con))
    {
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.Add("@Year", SqlDbType.Char, 4).Value = year;
        cmd.Parameters.Add("@startDate", SqlDbType.DateTime).Value = start;
        cmd.Parameters.Add("@endDate", SqlDbType.DateTime).Value = end;
        con.Open();

        DataSet dset = new DataSet();
        using (SqlDataAdapter adapter = new SqlDataAdapter(cmd))
        {
            adapter.Fill(dset);
        }
        this.gridDataSource.DataSource = dset.Tables[0];
    }
}
John Saunders
A: 

@Craig - "So, basically, "using" is the exact same as "Try/catch/finally" only much more flexible for error handling." That's not true, using is not fancy way to replace try/catch/finally, because it doesn't provide you with the way to "catch" exceptions. Using == Try/Finally (without the catch). If you still need to catch the exception and make some custom processing, you have to wrap/nest the using with try/catch block, or completely replace it with try/catch/finally.

devfreak