Importance de la fermeture des objets SQLConnection lors de l'utilisation de l'objet SQLDataReader

Question

Mon contrat actuel concerne une grande société de commerce électronique. Leur base de code dont les origines remontent à .Net 1.0 m'a pris par surprise pour contenir de nombreux problèmes qui augmentent le niveau d'odeur au-delà de la dernière merde que j'ai prise. Malgré cela et essayant de dissiper mon niveau de distraction, j'essaie gaiement d'ajouter des fonctionnalités pour résoudre d'autres problèmes ou étendre plus de conneries. Lorsque je touche le DAL/BLL, le temps nécessaire pour réparer ce qui précède sera effectué. Cependant, je voulais obtenir un vote de confiance de la part des experts pour avoir l'assurance de ne pas perdre de temps pour les clients ou pire d'avoir ma crédibilité rejetée en touchant «ce qui fonctionne». Bien sûr, les tests unitaires résoudraient ou au moins adouciraient cette inquiétude. Peut-être que cela devrait également être ajouté à 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 

Quelqu'un voit-il des problèmes avec ce côté de la ligne SQL qui rend mon désabonnement de l'intestin? À tout le moins, n'emballeriez-vous pas d'habitude ce qui précède dans un essai/attraper/finalement que la plupart d'entre nous sait a été autour depuis .Net v1.0? Encore mieux ne serait-il pas sage de corriger avec les instructions Using? La fermeture de SQLDataReader encapsule-t-elle vraiment la fermeture automatique de la connexion?

Answer 1

Rien ne cloche avec inline sql si l'entrée de l'utilisateur est correctement paramétrée, et cela ressemble à ça.

À part cela, oui, vous devez fermer les connexions. Sur un site web occupé, vous pourriez atteindre votre limite et cela causerait toutes sortes de bizarreries. J'ai également remarqué qu'il utilise toujours un arraylist. Depuis qu'ils sont passés de .Net 1.0 il est temps de mettre à jour ceux vers List<T> génériques (et éviter l'appel à CType - vous devriez pouvoir DirectCast() à la place).

Answer 2

Obtenez certainement des instructions using autour des objets Connection et Reader. S'il y a une exception, ils ne seront pas fermés tant que le Garbage Collector ne leur aura pas communiqué.

J'ai tendance à ne pas appeler .Close() lorsque des instructions sont utilisées. Même si le SqlDataReader ferme la connexion sur dispos (vérifiez le doco), mettre une utilisation autour de la connexion ne peut pas blesser et colle au modèle.

Si vous faites cela, l'essai/final n'est nécessaire que si vous devez effectuer une gestion des exceptions. J'ai tendance à laisser la gestion des exceptions aux niveaux supérieurs (envelopper chaque point d'entrée de l'interface utilisateur, les points d'entrée de la bibliothèque, les informations supplémentaires en exception) car la pile de données est généralement suffisante pour déboguer les erreurs.

Cela ne fait pas grand chose, mais si vous recréer du contenu, déplacez l'initialisation de la collection en dehors de la boucle. À la réflexion, le code renvoie null s'il n'y a pas d'enregistrements.

Au moins SqlParamètres sont utilisés! Débarrassez-vous de tout ce qui concatène l'entrée de l'utilisateur avec SQL si vous le trouvez (attaque par injection SQL), peu importe à quel point il est «nettoyé».

Answer 3

Si vous utilisiez C#, j'envelopperais la création du lecteur de données dans une instruction using, mais je ne pense pas que vb en possède?

Answer 4

La connexion sera fermée lorsque le lecteur est fermé car elle utilise le comportement de la commande CloseConnection.

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

Selon MSDN (http://msdn.microsoft.com/en-us/library/aa326246(VS.71).aspx)

Si le SqlDataReader est créé avec CommandBehavior réglé sur CloseConnection, la fermeture de la SqlDataReader ferme la connexion automatiquement.

Answer 5

En réponse à quelques-uns des grands points indiqués par Joel et Robert j'ai refactorisé la méthode comme suit qui a fonctionné sans faille.

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

créé une classe petite aide agréable pour envelopper les détails génériques jusqu'à

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 

Vous pouvez essayer quelque chose comme ce qui précède, les déclarations à l'aide manipuleront la fermeture de connexion et l'élimination des objets. Ceci est disponible lorsque la classe implémente IDisposable. Construisez et renvoyez aussi votre IList de CategoryInfo.