.Net ADO Connection Class

Question

I created a connection class that should return a datatables/datareaders etc to my webpages. I am worried that the connections won't be closed properly by using this class. Here is the class:

Imports Microsoft.VisualBasic

Namespace myConnection
  Public Class DB

      Public Shared Function GetConnStr()
                    Return "server=foobar"
      End Function


      Public Shared Function OpenConn()
                    Return New System.Data.SqlClient.SqlConnection( GetConnStr )
      End Function

      Public Shared Function OpenReader(SQL As String)

                    Dim conn 
                    conn = OpenConn
                    conn.Open

                    Return New System.Data.SqlClient.SqlCommand(SQL, conn).ExecuteReader(System.Data.CommandBehavior.CloseConnection)

      End Function

      Public Shared Function OpenTable(SQL As String)

                    Dim conn 
                    conn = OpenConn
                    conn.Open

                    Dim dr As System.Data.SqlClient.SqlDataReader = New System.Data.SqlClient.SqlCommand(SQL, conn).ExecuteReader(System.Data.CommandBehavior.CloseConnection)
                    Dim dt As System.Data.DataTable = New System.Data.DataTable()
          dt.Load(dr)

                    Return dt 

      End Function

      Public Shared Function ExecuteSQL(SQL As String)

                    Dim conn 
                    conn = OpenConn
                    conn.Open

                    Return New System.Data.SqlClient.SqlCommand(SQL, conn).ExecuteNonQuery()

      End Function


  End Class
End Namespace

And Here is how I am using it:

rst = conn.OpenReader(SQL)
While rst.Read  
end while
rst.close

I am worried that once I go into production the connections won't be close properly and my site will fail. I am new to .net, is there anything wrong with the principal behind this class?

It's OK to write your Connection Helper class, as long as you remember to always explicitly close and dispose your connection wherever you're using it, not just on your base class. — Smur, Apr 25 '11 at 14:30
Andomar, can you elaborate on why it's a bad idea? Felipe, in my example would rst.Close be enough or would I have to do rst.Close and rst.Dispose ? — FatThumbs, Apr 25 '11 at 14:33
The Close() method does close the connection but it leaves the object in memory in case it'll be used later, the Dipose() method eliminates the object and its resources, so it can not be used anymore. — Smur, Apr 25 '11 at 15:01

score 2 · Accepted Answer · edited May 23 '17 at 12:18

You are right: your connection won't be closed this way. Even worse, by only accepting strings for your sqlcommand you open yourself up to sql injection security vulnerabilities. As an example of a better pattern, the code I use to fill a data table looks more like this:

Public Function GetDataTable(ByVal sql As String, ByVal AddParameters As Action(Of SqlParameterCollection)) As DataTable
    Dim result As New DataTable()
    Using cn As SqlConnection = OpenConn(), _
          cmd As New SqlCommand(sql, cn)

        AddParameters(cmd.Parameters)

        Using rdr As SqlDataReader = cmd.ExecuteReader
            result.Load(rdr)
        End Using
    End Using
    Return result
End Function

I would then call the code like this:

Dim data As DataTable = GetDataTable("SELECT * FROM SomeTable WHERE ID= @ID", _ 
       Sub(p)
           p.Add("@ID", SqlDbType.Int).Value = 12345
       End Sub )

I have similar code in C# for an SqlDataReader, but it requires using an iterator block and that feature ~~is not available for VB~~was only just added to VB.Net with the service pack for visual studio 2010 and Async CTP out a few weeks ago. The important thing to take away here is that I have the sql connection correctly encapsulated with a Using block and the code encourages the correct use of query parameters.

score 0 · Answer 2 · answered Apr 25 '11 at 15:52

Unfortunately, I agree with one of the other comments. Why are you writing your own connection classes?

Use ADO.NET EF or LINQ To SQL which will manage the connections in the Context.

If you do continue to do what you are doing, wrap your connection in a Using block.

score 0 · Answer 3 · answered Apr 27 '11 at 13:55

I use a module to call the database, saves alot of lines if your usin multiple forms...

This is my module form:

Public cn As OleDbConnection

Public Sub InitDatabase()
    Dim sDBase As String = "DB.mdb"
    cn= New OleDbConnection
    cn.ConnectionString = "Provider=Microsoft.JET.OLEDB.4.0; Data Source=" & sDBase
End Sub

then for callin the database use this:

Private ds As New DataSet
Private da As New OleDbDataAdapter

modWijnen.InitDatabase()

    Dim cm As New OleDbCommand("Select * from Table1", cn)
    da = New OleDbDataAdapter(cm)

    If (ds.Tables.Contains("Table1") = False) Then
        da.Fill(ds, "Table1")
    End If

I hope this has been helpfull for you...

.Net ADO Connection Class

3 Answers3