How to Close a DataReader on Exception

Question

I have the following code in some methods of my Data Layer:

StringBuilder sb = new StringBuilder();
SqlCommand s = new SqlCommand(sb.ToString(), conn);
try 
{ 
    SqlDataReader dr = s.ExecuteReader(); 
    while(dr.Read())
      DoSomething(dr);
}
catch (Exception ex)
{ 
    sb.Append(Util.ExceptionRecursive(ex)); 
}

The thing is, dr never closes in case of exception. And when other method tries to access another data reader, it throws another exception that says something like "Another Datareader is connected to the Database"

I want to close my DataReader in any case. But this:

sb = new StringBuilder();
SqlCommand s = new SqlCommand(sb.ToString(), conn);
SqlDataReader dr;
try 
{
    dr = s.ExecuteReader(); 
    while(dr.Read())
      DoSomething(dr);
}
catch (Exception ex)
{ 
    sb.Append(Util.ExceptionRecursive(ex)); 
}
finally
{
    dr.Close();
}

Won't work because in case of exception dr may have no data, and won't compile.

How should I do it then?

Fable

You should use a ConnectionStringBuilder EDIT: (if you're building a connection string) – Shellashellac

@Lasse: For the connection string. (I thought he was creating a SqlConnection too) – Shellashellac

You should probably be using parameters – Shellashellac

conn is a SqlConnection, already made. – Fable

Answer 1

You should use the using statement:
It generates a finally block to ensure that your resource is always disposed.

StringBuilder sb = new StringBuilder();
using (SqlCommand s = new SqlCommand(sb.ToString(), conn)) {
    try 
    { 

        using (SqlDataReader dr = s.ExecuteReader()) {
            while(dr.Read())
              DoSomething(dr);
        }

    }
    catch (Exception ex)
    { 
        sb.Append(Util.ExceptionRecursive(ex)); 
    }    
}

Shellashellac

what internally would the using do? Dispose or Close? If it's Dispose, will it "remember" to Close the DataReader? – Fable

@apacay: Dispose() calls Close(). You should use a separate using statement per object. – Shellashellac

Answer 2

The best way is probably this:

sb = new StringBuilder();
...
using (SqlCommand s = new SqlCommand(sb.ToString(), conn))
using (SqlDataReader dr = s.ExecuteReader())
{
    try
    {
        while(dr.Read())
          DoSomething(dr);
    }
    catch (Exception ex)
    { 
        sb.Append(Util.ExceptionRecursive(ex)); 
    }
}

However, if you're expecting (or not) exceptions during SQL execution, you must place the exception handling code outside:

sb = new StringBuilder();
...
try
{
    using (SqlCommand s = new SqlCommand(sb.ToString(), conn))
    using (SqlDataReader dr = s.ExecuteReader())
    {
        while(dr.Read())
          DoSomething(dr);
    }
}
catch (Exception ex)
{ 
    sb.Append(Util.ExceptionRecursive(ex)); 
}

Zajac

But it all depends on where you expect exceptions. – Zajac

Exceptions generally come from ExecuteReader, not Read. – Shellashellac

They could come from DoSomething, but I agree, edited the answer – Zajac

@Lasse my problem comes from the ExecuteReader... DoSomething(SqlDataReader dr) shouldn't cause any trouble. – Fable

Answer 3

you can write it as:

if(dr!=null) dr.Close();

Barograph

I thought of that but it seemed unclean clean to me. – Fable