Важность закрытия объектов SQLConnection при использовании объекта SQLDataReader

Question

Мое настоящее контрактное соглашение связано с крупной компанией электронной коммерции. Их кодовая база, берущая начало в .Net 1.0, застала меня врасплох, так как содержит много проблем, которые поднимают уровень запаха за пределы последнего дерьма, которое я взял.

Несмотря на то, что, пытаясь отвлечься от этого, я стараюсь изо всех сил пытаться добавить новые функции, чтобы либо исправить другие проблемы, либо расширить дерьмо. Когда я коснусь DAL / BLL, будет сделано время, необходимое для исправления вышеупомянутого. Однако я хотел получить вотум доверия экспертов, чтобы получить некоторую уверенность в том, что я не трачу время клиентов или что еще хуже, если мой авторитет будет отвергнут, коснувшись «вещей, которые работают». Конечно, юнит-тестирование решило бы или хотя бы смягчило это беспокойство. Возможно, это также следует добавить в wtf.com?

Public Function GetSizeInfoBySite(ByVal siteID As String) As IList
    Dim strSQL As String = "YES INLINE SQL!! :)"
    Dim ci As CrapInfo
    Dim alAnArrayList As ArrayList

    Dim cn As New SqlConnection(ConfigurationSettings.AppSettings("ConnectionString"))
    Dim cmd As New SqlCommand(strSQL, cn)
    cmd.Parameters.Add(New SqlParameter("@MySiteID", SqlDbType.NVarChar, 2)).Value = siteID
    cn.Open()
    Dim rs As SqlDataReader = cmd.ExecuteReader(CommandBehavior.CloseConnection)
    While rs.Read()
        ci = New CategoryInfo(rs("someID"), rs("someName"))
        If IsNothing(alAnArrayList) Then
            alAnArrayList = New ArrayList
        End If
        alAnArrayList.Add(ci)
    End While
    rs.Close()
    Return CType(alAnArrayList, IList)
End Function

Кто-нибудь видит проблемы с этим, кроме встроенного SQL, который заставляет меня нервничать? По крайней мере, разве вы не обернули бы вышеизложенное в try / catch / finally, которое большинство из нас знает, начиная с .Net v1.0? Еще лучше, не было бы разумно исправить с помощью операторов? Действительно ли закрытие SQLDataReader автоматически закрывает соединение?

Answer 1

Ничего плохого в встроенном SQL, если пользовательский ввод правильно параметризован, и это выглядит так.

Кроме этого, да, вам нужно закрыть соединения. На загруженном веб-сайте вы можете достичь своего предела, и это вызовет все виды странностей.

Я также заметил, что он все еще использует arraylist. Поскольку они перешли с .Net 1.0, пришло время обновить их до List<T> (и избежать вызова CType - вы должны иметь возможность вместо этого DirectCast ()).

Answer 2

Соединение будет закрыто, когда читатель закрыт, потому что он использует поведение команды CloseConnection.

Dim rs As SqlDataReader = cmd.ExecuteReader(CommandBehavior.CloseConnection)

По данным MSDN (http://msdn.microsoft.com/en-us/library/aa326246(VS.71).aspx)

Если SqlDataReader создан с CommandBehavior, установленным в значение CloseConnection, закрытие SqlDataReader автоматически закрывает соединение.

Answer 3

Определенно получите несколько операторов using для объектов Connection и Reader. Если есть исключение, они не будут закрыты до тех пор, пока к ним не придет сборщик мусора.

Я склонен не вызывать .Close (), когда используются операторы. Даже если SqlDataReader закрывает соединение при утилизации (проверьте документ), использование этого соединения не повредит и не нарушит шаблон.

Если вы сделаете это, попытка / окончание необходима только в том случае, если вам нужно выполнить обработку исключений прямо здесь. Я склонен оставлять обработку исключений на более высоких уровнях (обернуть каждую точку входа пользовательского интерфейса, точки входа библиотеки, дополнительную информацию в исключении), поскольку трассировки стека обычно достаточно для отладки ошибок.

Не то чтобы это имело большое значение, но если вы рефакторинг, переместите инициализацию коллекции за пределы цикла. Если подумать, код возвращает ноль, если нет записей.

Используются хотя бы SqlParameters! Избавьтесь от всего, что объединяет пользовательский ввод с SQL, если вы его обнаружите (атака SQL Injection), независимо от того, насколько хорошо оно «очищено».

Answer 4

В ответ на некоторые из замечательных моментов, указанных Джоэлом и Робертом I, рефакторинг метода был следующим, который работал безупречно.

Public Function GetSomeInfoByBusObject(ByVal SomeID As String) As IList
Dim strSQL As String = "InLine SQL"
Dim ci As BusObject
Dim list As New GenList(Of BusObject)
Dim cn As New SqlConnection(
    ConfigurationSettings.AppSettings("ConnectionString"))
Using cn
    Dim cmd As New SqlCommand(strSQL, cn)
    Using cmd
        cmd.Parameters.Add(New SqlParameter
            ("@SomeID", SqlDbType.NVarChar, 2)).Value = strSiteID
        cn.Open()
            Dim result As SqlDataReader = cmd.ExecuteReader(CommandBehavior.CloseConnection)
                While result.Read()
                    ci = New BusObject(rs("id), result("description"))
                    list.Add(DirectCast(ci, BusObject))
                End While
            result.Close()
        End Using
    Return list
End Using

Функция завершения

Создан хороший класс помощника, чтобы обернуть общие детали

Public Class GenList(Of T)
    Inherits CollectionBase
    Public Function Add(ByVal value As T) As Integer
        Return List.Add(value)
    End Function
    Public Sub Remove(ByVal value As T)
        List.Remove(value)
    End Sub
    Public ReadOnly Property Item(ByVal index As Integer) As T
        Get
            Return CType(List.Item(index), T)
        End Get
    End Property
End Class

Answer 5

Public Function GetSizeInfoBySite(ByVal siteID As String) As IList(Of CategoryInfo)
        Dim strSQL As String = "YES INLINE SQL!! :)"

        'reference the 2.0 System.Configuration, and add a connection string section to web.config
        '  <connectionStrings>
        '   <add name="somename" connectionString="someconnectionstring" />
        '  </connectionStrings >

        Using cn As New SqlConnection(System.Configuration.ConfigurationManager.ConnectionStrings("somename").ConnectionString

            Using cmd As New SqlCommand(strSQL, cn)

                cmd.Parameters.Add(New SqlParameter("@MySiteID", SqlDbType.NVarChar, 2)).Value = siteID
                cn.Open()

                Using reader As IDataReader = cmd.ExecuteReader()

                    Dim records As IList(Of CategoryInfo) = New List(Of CategoryInfo)

                    'get ordinal col indexes
                    Dim ordinal_SomeId As Integer = reader.GetOrdinal("someID")
                    Dim ordinal_SomeName As Integer = reader.GetOrdinal("someName")

                    While reader.Read()
                        Dim ci As CategoryInfo = New CategoryInfo(reader.GetInt32(ordinal_SomeId), reader.GetString(ordinal_SomeName))
                        records.Add(ci)
                    End While

                    Return records

                End Using
            End Using
        End Using
    End Function

Вы можете попробовать что-то подобное, операторы using будут обрабатывать закрытие соединения и удаление объекта. Это доступно, когда класс реализует IDisposable. Также создайте и верните свой IList для CategoryInfo.

Answer 6

Если бы вы использовали c #, я бы обернул создание заголовка данных в оператор using, но я не думаю, что vb его имеет?