Writing a Data Access Layer

I am new to programming and have written this Data Access Layer. I am using this DAL in my projects. Now, I have a feeling that it is pathetic (when I wrote it, I was kind of happy). Do you guys also feel the same for this DAL. Kindly help me write a better one. There is no Transaction in my DAL and I have no idea how to implement it in this DAL. I am expecting some criticism.

Connections.vb Class File

Option Explicit On
Option Strict On
Imports Microsoft.VisualBasic
Imports System.Data.SqlClient

Public Class Connection
    Public Shared Function GetDbConnection() As SqlConnection
        Dim _conString As String = ConfigurationManager.ConnectionStrings("CoachingConnectionString").ConnectionString
        Dim _connection As New SqlConnection(_conString)
        _connection.Open()
        Return _connection
    End Function
End Class

DataAccess.vb Class File

```
Option Explicit On
Option Strict On
Imports Microsoft.VisualBasic
Imports System.Data.SqlClient
Imports System.Data

Public Class DataAccess
Public Overloads Shared Function GetDataTable(ByVal _sql As String, ByVal _parameterNames() As String, ByVal _parameterVals() As String) As DataTable
Dim _connection As SqlConnection = Global.Connection.GetDbConnection()
Dim _command As New SqlDataAdapter(_sql, _connection)
Dim _table As New DataTable
Try
If _parameterNames IsNot Nothing Then
For i = 0 To _parameterNames.Length - 1
_command.SelectCommand.Parameters.AddWithValue(_parameterNames(i), _parameterVals(i))
Next
End If
_command.Fill(_table)
Catch ex As Exception
'MsgBox(ex.Message)
_table = Nothing
Finally
If _connection.State = ConnectionState.Open Then
_connection.Close()
_connection.Dispose()
_command.Dispose()
End If
End Try

Return _ta

The first part of the answer has already been given by Leonid: "don't create your own data access methodology". Use something like Entity Framework. This is a frequently-used "wheel": don't reinvent it.

That said, there are a number of issues with your code:

• You do not need a separate class for your GetDbConnection function. Put it into your DataAccess class and make it Private.

• The SqlConnection and SqlDataAdapter classes implement the IDisposable interface. When you create an object of such a type, and consume it entirely within your method, you should place the object in a Using block. This ensures it gets cleaned up as you are doing in your Finally.

• Given #2, you don't need Try/Catch blocks at all. In your case, they have the primary effect of hiding any exceptions that may be thrown, so that you will never know what's wrong with your code.

• You have duplicated the code for filling the parameters. Extract that into a separate method.

• I recommend that you use a leading "_" only for class-level fields, and not for the parameters to a Function or Sub.

Here's the result of a quick refactoring of your code to resolve those issues:

Option Explicit On
Option Strict On

Imports System.Data.SqlClient
Imports System.Configuration

Public Class DataAccess
    Public Overloads Shared Function GetDataTable(ByVal sql As String, ByVal parameterNames() As String, ByVal parameterVals() As String) As DataTable
        Using connection As SqlConnection = GetDbConnection()
            Using da As SqlDataAdapter = New SqlDataAdapter(sql, connection)
                Dim table As New DataTable
                FillParameters(da.SelectCommand, parameterNames, parameterVals)
                da.Fill(table)
                Return table
            End Using
        End Using
    End Function

    Public Overloads Shared Function GetDataTable(ByVal sql As String) As DataTable
        Using connection As SqlConnection = GetDbConnection()
            Using da As New SqlDataAdapter(sql, connection)
                Dim table As New DataTable
                da.Fill(table)
                Return table
            End Using
        End Using
    End Function

    Public Shared Function SelectScalar(ByVal sql As String, ByVal parameterNames() As String, ByVal parameterVals() As String) As String
        Using connection As SqlConnection = GetDbConnection()
            Using command As SqlCommand = New SqlCommand(sql, connection)
                FillParameters(command, parameterNames, parameterVals)
                Return CStr(command.ExecuteScalar)
            End Using
        End Using
    End Function

    Public Shared Function SelectScalar(ByVal sql As String) As String
        Using connection As SqlConnection = GetDbConnection()
            Using command As New SqlCommand(sql, connection)
                Return CStr(command.ExecuteScalar)
            End Using
        End Using
    End Function

    Public Shared Function CRUD(ByVal sql As String, ByVal parameterNames() As String, ByVal parameterVals() As String) As Integer
        Using connection As SqlConnection = GetDbConnection()
            Using command As New SqlCommand(sql, connection)
                FillParameters(command, parameterNames, parameterVals)
                Return command.ExecuteNonQuery()
            End Using
        End Using
    End Function

    Private Shared Sub FillParameters(ByVal command As SqlCommand, ByVal parameterNames As String(), ByVal parameterVals As String())
       If parameterNames IsNot Nothing Then
            For i = 0 To parameterNames.Length - 1
                command.Parameters.AddWithValue(parameterNames(i), parameterVals(i))
            Next
        End If
    End Sub

    Private Shared Function GetDbConnection() As SqlConnection
        Dim conString As String = ConfigurationManager.ConnectionStrings("CoachingConnectionString").ConnectionString
        Dim connection As SqlConnection = New SqlConnection(conString)
        connection.Open()
        Return connection
    End Function
End Class

StackExchange Code Review Q#15063, answer score: 7