Важность закрытия объектов 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

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

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

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

Не важно, что это важно, но если вы перефакторизуете, переместите инициализацию коллекции вне цикла. Во-вторых, код возвращает null, если нет записей.

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

Answer 3

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

Answer 4

Соединение закрывается, когда считыватель закрыт, потому что он использует поведение команды 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 5

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

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 

End Function

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

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 6

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 

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