4
\$\begingroup\$

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 _table
    End Function

    Public Overloads Shared Function GetDataTable(ByVal _sql As String) As DataTable
        Dim _connection As SqlConnection = Global.Connection.GetDbConnection()
        Dim _command As New SqlDataAdapter(_sql, _connection)
        Dim _table As New DataTable
        Try
            _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 _table
    End Function

    Public Shared Function SelectScalar(ByVal _sql As String, ByVal _parameterNames() As String, ByVal _parameterVals() As String) As String
        Dim _returnVal As String
        Dim _connection As SqlConnection = Global.Connection.GetDbConnection()
        Dim _command As New SqlCommand(_sql, _connection)
        Dim _value As String

        Try
            If _parameterNames IsNot Nothing Then
                For i = 0 To _parameterNames.Length - 1
                    _command.Parameters.AddWithValue(_parameterNames(i), _parameterVals(i))
                Next
            End If

            _value = CStr(_command.ExecuteScalar)
            _returnVal = _value
        Catch ex As Exception
            _returnVal = Nothing
        Finally
            If _connection.State = ConnectionState.Open Then
                _connection.Close()
                _connection.Dispose()
                _command.Dispose()
            End If
        End Try

        Return _returnVal
    End Function
    Public Shared Function SelectScalar(ByVal _sql As String) As String
        Dim _returnVal As String
        Dim _connection As SqlConnection = Global.Connection.GetDbConnection()
        Dim _command As New SqlCommand(_sql, _connection)
        Dim _value As String

        Try


            _value = CStr(_command.ExecuteScalar)
            _returnVal = _value
        Catch ex As Exception
            _returnVal = Nothing
        Finally
            If _connection.State = ConnectionState.Open Then
                _connection.Close()
                _connection.Dispose()
                _command.Dispose()
            End If
        End Try

        Return _returnVal
    End Function
    Public Shared Function CRUD(ByVal _sql As String, ByVal _parameterNames() As String, ByVal _parameterVals() As String) As Integer
        Dim _noOfRowsAffected As Integer
        Dim _connection As SqlConnection = Global.Connection.GetDbConnection()
        Dim _command As New SqlCommand(_sql, _connection)

        Try
            If _parameterNames IsNot Nothing Then
                For i = 0 To _parameterNames.Length - 1
                    _command.Parameters.AddWithValue(_parameterNames(i), _parameterVals(i))
                Next
            End If

            _noOfRowsAffected = _command.ExecuteNonQuery()
        Catch ex As Exception
            'MsgBox(ex.Message)
            _noOfRowsAffected = -1
        Finally
            If _connection.State = ConnectionState.Open Then
                _connection.Close()
                _connection.Dispose()
                _command.Dispose()
            End If
        End Try

        Return _noOfRowsAffected
    End Function
End Class
\$\endgroup\$
5
  • 2
    \$\begingroup\$ Why not use ORM / MVC / EF ? \$\endgroup\$
    – Leonid
    Aug 25, 2012 at 18:17
  • \$\begingroup\$ Actually am using ASP.NET web forms and new to programming...so i hope you understand..what is ORM..and kindly point out the major cons with my DAL so that i can learn what are my mistakes. \$\endgroup\$ Aug 25, 2012 at 18:21
  • \$\begingroup\$ Watch this whole video for a hint - the Object Relational Mapping (ORM) is taken care of by the Entity Framework youtube.com/watch?v=2AWVs7SzOXM&feature=related \$\endgroup\$
    – Leonid
    Aug 25, 2012 at 19:03
  • \$\begingroup\$ @Leonid Thanks for the link...i am watching it now. \$\endgroup\$ Aug 25, 2012 at 19:13
  • \$\begingroup\$ you can create <en.wikipedia.org/wiki/Strategy_pattern> if you have more than one database to connect, so that you can dynamically change your strategy (database type like msaccess, mysql, oracle etc). I recommend this approach. \$\endgroup\$
    – rykhan
    Mar 20, 2013 at 9:54

1 Answer 1

7
\$\begingroup\$

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:

  1. You do not need a separate class for your GetDbConnection function. Put it into your DataAccess class and make it Private.
  2. 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.
  3. 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.
  4. You have duplicated the code for filling the parameters. Extract that into a separate method.
  5. 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
\$\endgroup\$
2
  • \$\begingroup\$ Which ORM would you recommend?? I want something easy to learn with not a very steep learning curve. Which one would you prefer ?? Microsoft Enterprise Library 5.0 or MS Entity Framework or NHibernate. Any other suggestions? \$\endgroup\$ Aug 26, 2012 at 15:59
  • 1
    \$\begingroup\$ Entity Framework. It's from Microsoft. Enterprise Library is not an ORM. \$\endgroup\$ Aug 26, 2012 at 17:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.