1

Ok, If I put a dbDataReader in a "Using" statement do I still need to explicitly call the dbDataReader.Close. In the .net documentation it says that "Using" a connection automatically Closes it. In the example below from MSDN it shows a dbDataReader without a "Using" and explicitly closing it: 

Public Sub ReadData(ByVal connectionString As String)
    Dim queryString As String = _
       "SELECT OrderID, CustomerID FROM Orders" 

    Using connection As New OdbcConnection(connectionString)
       Dim command As New OdbcCommand(queryString, connection)
       connection.Open()

       Dim reader As OdbcDataReader
       reader = command.ExecuteReader()

       ' Always call Read before accessing data. 
       While reader.Read()
          Console.WriteLine(reader.GetInt32(0) & ", " & reader.GetString(1))
       End While 

       ' Always call Close when done reading.
       reader.Close()
    End Using 
End Sub

So isn't this cleaner and more efficient:

Public Sub ReadData(ByVal connectionString As String)
    Dim queryString As String = _
    "SELECT OrderID, CustomerID FROM Orders"

    Using connection As New OdbcConnection(connectionString)
        Using command As New OdbcCommand(queryString, connection)
            connection.Open()

            Using reader = command.ExecuteReader()

                ' Always call Read before accessing data. 
                While reader.Read()
                    Console.WriteLine(reader.GetInt32(0) & ", " & reader.GetString(1))
                End While

            End Using
        End Using
    End Using
End Sub

And then you wouldn't need to explicitly call the .Close on the Reader?

Thanks in advance

dblwizard
  • 595
  • 7
  • 26

3 Answers3

0

In the code snippet you are using, you don't need to explicitly call .Close on the reader. However, it's a good habit to get into. Explicitly close the reader and the connection as soon as you are finished using them.

Al W
  • 7,539
  • 1
  • 19
  • 40
  • So "Best Practice" would be to add the connection.Close and reader.Close before each respective "End Using"? – dblwizard May 30 '14 at 16:42
  • Close is 100% redundant to Dispose. You'd need to argue why to call both! – usr May 31 '14 at 15:38
  • Best practice would be to close the reader and connection as soon as you can. If it is right before the end using, then, just as @usr says, it is redundant, but it doesn't take much imagination to see a scenario where you might be able to close it, but not be ready to dispose it yet. – Al W Jun 01 '14 at 18:54
  • @AlW, I could see a scenario but it would be very rare and usually a sign that there is a better way to do it. If you are doing very much inside the loop where you are reading data then you probably are doing it in the wrong place. – dblwizard Jun 02 '14 at 20:27
-1

For me best practice is neither
A try, catch, finally to handle any SQL exceptions 

SqlConnection sqlConnRW = new SqlConnection(sqlConnStringLibDef);
try
{
    sqlConnRW.Open();
    SqlCommand sqlCmd = sqlConnRW.CreateCommand();
    sqlCmd.CommandText = "select column from table";
    SqlDataReader rdr = sqlCmd.ExecuteReader();
    while (rdr.Read())
    { }
    rdr.Close();
}
catch (SqlException Ex)
{
    Debug.WriteLine(Ex.Message);
    Debug.WriteLine(Ex.Number);
}
finally
{
    sqlConnRW.Close();
}
paparazzo
  • 44,497
  • 23
  • 105
  • 176
  • Are there objective reasons to prefer this much longer, yet equivalent, style to `using`? I have trouble coming up with any. – usr May 31 '14 at 15:42
  • @usr Equivalent? Using does not have a catch. You down voted this because you could not see a difference? See line 2. A catch to handle SQL exceptions – paparazzo May 31 '14 at 17:26
  • Try-catch is compatible with `using`. Just use both. (I am indeed the downvoter.) – usr May 31 '14 at 22:13
  • @usr Pick a story. First you state equivalent now you state compatible. – paparazzo Jun 01 '14 at 00:06
  • @usr Pick an objective. Use both would be longer not shorter. – paparazzo Jun 01 '14 at 00:14
  • @Blam I could see this if you needed specific exception handling at this level, but typically I leave the default exception handler or have exception handling at a higher level than this. – dblwizard Jun 02 '14 at 20:26
  • @dblwizard Really that is your practice - no need for specific handling at this level - leave it for the default exception handler. That is just plain sloppy. You have a SQL command and you see no need to handle SqlException. Really no need to report details like server down, credentials invalid, timeout, constraint violation... So you just let the app crash out. But let the user rest assured that you did not hold any resource as you used a using statement. – paparazzo Jun 02 '14 at 21:19
  • @Blam I didn't say I only use the default exception handler. I also said "..or have exception handling at a higher level". By default exception handler I did not mean just let the program end. We have a defined default exception handler that logs and reports errors. All our exceptions in production cause a notification, not just a log entry so ANY exception is going to get attention, it includes the complete call stack and other exception information. – dblwizard Jun 04 '14 at 01:07
-1

Close is 100% redundant to Dispose. using calls Dispose in a safe way. Just always put your resources in a using block. It is made for this purpose. It is safe by construction. It is easy to inspect and immediately see that it is correct.

No need to call Close. It is redundant and confusing. Just always follow the using habit if you can.

Resource handling in C# and VB.NET is very easy. Just use using!

If you require error handling, just add a try-catch inside or outside the using.

usr
  • 168,620
  • 35
  • 240
  • 369
  • In a negative comment to my answer you stated using and try catch are equivalent. If they are equivalent then why would you need a try catch in a using. – paparazzo Jun 01 '14 at 00:07
  • @Blam that was a typo. I meant try-finally. I stand by the idea behind these comments. – usr Jun 01 '14 at 10:37