Data Access Layer code for MSSQL Databases

We use this class quite a bit for boilerplate / glue that allows web services and applications to call databases. This code is very important, and I'd like to ensure I'm getting the best performance possible.

Any and all feedback is welcome! If there is a better / more efficient way of doing something, please let me know.

`Imports System.Data.SqlClient
Imports System.Data

Public Class MSSqlUtility

#Region "Data Access Utilities"

'''
''' Gets the data table.
'''
''' The _command.
''' Type of the _command.
''' The parameter list.
''' DataTable object - returns empty datatable if it failed
Public Shared Function GetDataTable(ByVal _command As String, ByVal _commandType As CommandType, Optional ByVal _prmList As ArrayList = Nothing) As DataTable

Dim cmd As SqlCommand = Nothing
Dim conn As SqlConnection = Nothing
Try
Using dt As New DataTable()
conn = New SqlConnection(GetConnectionString())
cmd = New SqlCommand(_command, conn)
cmd.CommandType = _commandType

If (_prmList IsNot Nothing) Then
For Each p As SqlParameter In _prmList
cmd.Parameters.Add(p)
Next
End If

If (conn.State <> ConnectionState.Open) Then
conn.Open()
End If
'Fill the datatable object
Using adapter As New SqlDataAdapter(cmd)
adapter.Fill(dt)
End Using
If (conn.State <> ConnectionState.Closed) Then
conn.Close()
End If
Return dt
End Using
Catch ex As Exception
LoggingUtility.LogError(ex, "MSSqlUtility.GetDataTable")
Return New DataTable()
Finally
cmd.Connection.Close()
cmd.Parameters.Clear()
cmd.Dispose()

This whole function needs a little bit of work:

Dim cmd As SqlCommand = Nothing
    Dim conn As SqlConnection = Nothing
    Try
        Using dt As New DataTable()
            conn = New SqlConnection(GetConnectionString())
            cmd = New SqlCommand(_command, conn)
            cmd.CommandType = _commandType

            If (_prmList IsNot Nothing) Then
                For Each p As SqlParameter In _prmList
                    cmd.Parameters.Add(p)
                Next
            End If

            If (conn.State <> ConnectionState.Open) Then
                conn.Open()
            End If
            Using adapter As New SqlDataAdapter(cmd)
                adapter.Fill(dt)
            End Using
            If (conn.State <> ConnectionState.Closed) Then
                conn.Close()
            End If
            Return dt
        End Using
    Catch ex As Exception
        LoggingUtility.LogError(ex, "MSSqlUtility.GetDataTable")
        Return New DataTable()
    Finally
        cmd.Connection.Close()
        cmd.Parameters.Clear()
        cmd.Dispose()
        conn.Dispose()
    End Try

you should add some using statements here, like a lot of them.

First thing is that we use more using statements and get rid of the finally statement, like this:

Dim cmd As SqlCommand = Nothing
    Dim conn As SqlConnection = Nothing
    Try
        Dim dt As New DataTable()
        Using conn As New SqlConnection(GetConnectionString())
            Using cmd = New SqlCommand(_command, conn)
                cmd.CommandType = _commandType

                If (_prmList IsNot Nothing) Then
                    For Each p As SqlParameter In _prmList
                        cmd.Parameters.Add(p)
                    Next
                End If

                If (conn.State <> ConnectionState.Open) Then
                    conn.Open()
                End If
                Using adapter As New SqlDataAdapter(cmd)
                    adapter.Fill(dt)
                End Using
                If (conn.State <> ConnectionState.Closed) Then
                    conn.Close()
                End If
                Return dt
            End Using
        End Using
    Catch ex As Exception
        LoggingUtility.LogError(ex, "MSSqlUtility.GetDataTable")
        Return New DataTable()
    End Try

Then we are going to get rid of:

If (conn.State <> ConnectionState.Open) Then
     conn.Open()
 End If

and this:

If (conn.State <> ConnectionState.Closed) Then
    conn.Close()
End If

They are not needed, at all. In fact, you don't even need to manually close anything declared with Using.

Now we have this:

Dim cmd As SqlCommand = Nothing
Dim conn As SqlConnection = Nothing
Try
    Dim dt As New DataTable()
    Using conn As New SqlConnection(GetConnectionString(
        Using cmd = New SqlCommand(_command, conn)
            cmd.CommandType = _commandType

            If (_prmList IsNot Nothing) Then
                For Each p As SqlParameter In _prmList
                    cmd.Parameters.Add(p)
                Next
            End If

            Using adapter As New SqlDataAdapter(cmd)
                adapter.Fill(dt)
            End Using
            Return dt
        End Using
    End Using
Catch ex As Exception
    LoggingUtility.LogError(ex, "MSSqlUtility.GetDataTable")
    Return New DataTable()
End Try

Wash, Rinse, and Repeat until all 3 look this clean, please.

I took another answers advice and removed the using block for the DataTable that is being returned. You may want to look into another way of returning the data in that DataTable though, as passing around something that is disposable like that can cause connection issues.

StackExchange Code Review Q#85506, answer score: 11