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?

You should use a `ConnectionStringBuilder` **EDIT**: (if you're building a connection string) — SLaks, Jul 05 '11 at 20:17
@Lasse: For the connection string. (I thought he was creating a SqlConnection too) — SLaks, Jul 05 '11 at 20:20

score 12 · Accepted Answer · answered Jul 05 '11 at 20:15

12

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)); 
    }    
}

answered Jul 05 '11 at 20:15

SLaks

868,454
176
1,908
1,964

what internally would the using do? Dispose or Close? If it's Dispose, will it "remember" to Close the DataReader? – apacay Jul 05 '11 at 20:35
2

@apacay: `Dispose()` calls `Close()`. You should use a separate `using` statement per object. – SLaks Jul 05 '11 at 20:44

score 4 · Answer 2 · answered Jul 05 '11 at 20:16

4

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)); 
}

answered Jul 05 '11 at 20:16

Lasse V. Karlsen

380,855
102
628
825

But it all depends on where you expect exceptions. – Lasse V. Karlsen Jul 05 '11 at 20:17
Exceptions generally come from `ExecuteReader`, not `Read`. – SLaks Jul 05 '11 at 20:18
They could come from `DoSomething`, but I agree, edited the answer – Lasse V. Karlsen Jul 05 '11 at 20:18
@Lasse my problem comes from the ExecuteReader... `DoSomething(SqlDataReader dr)` shouldn't cause any trouble. – apacay Jul 05 '11 at 20:30

score 0 · Answer 3 · answered Jul 05 '11 at 20:16

0

you can write it as:

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

answered Jul 05 '11 at 20:16

Dmitry

3,069
1
17
26

I thought of that but it seemed unclean clean to me. – apacay Jul 05 '11 at 20:31

How to Close a DataReader on Exception

3 Answers3