How to Close a DataReader on Exception

c# .net-1.1 sqldatareader sqlcommand

24,326

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

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

24,326

Author by

apacay

C# Developer, PM, Software architect, Table RPG player(?)

Updated on July 06, 2020

Comments

apacay almost 4 years
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?
- SLaks almost 13 years
  
  You should use a ConnectionStringBuilder EDIT: (if you're building a connection string)
- SLaks almost 13 years
  
  @Lasse: For the connection string. (I thought he was creating a SqlConnection too)
- SLaks almost 13 years
  
  You should probably be using parameters
- apacay almost 13 years
  
  conn is a SqlConnection, already made.
Lasse V. Karlsen almost 13 years

But it all depends on where you expect exceptions.
SLaks almost 13 years

Exceptions generally come from ExecuteReader, not Read.
Lasse V. Karlsen almost 13 years

They could come from DoSomething, but I agree, edited the answer
apacay almost 13 years

@Lasse my problem comes from the ExecuteReader... DoSomething(SqlDataReader dr) shouldn't cause any trouble.
apacay almost 13 years

I thought of that but it seemed unclean clean to me.
apacay almost 13 years

what internally would the using do? Dispose or Close? If it's Dispose, will it "remember" to Close the DataReader?
SLaks almost 13 years

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