Separating presentation/BLL/DAL

Question

I'm new to ASP.Net and up until now my code has worked but has been a mish mash all over the place with no real structure. I'm trying to teach myself how to keep the presentation/BLL/DAL separate so I have built a simple page to try and work with.

Page has 3 text boxes: clock number, first name and last name. On inputting a clock number in the clock number text box, I want to go off to my database and fetch the first name and last name of the person that clock number belongs to.

My DAL consists of 3 classes as below:

Public Class EmployeeName
Private _FirstName As String
Private _Lastname As String

Public Property FirstName() As String
    Get
        Return _FirstName
    End Get
    Set(value As String)
        _FirstName = value
    End Set
End Property

Public Property LastName() As String
    Get
        Return _Lastname
    End Get
    Set(value As String)
        _Lastname = value
    End Set
End Property
End Class

Public Class EmployeeNameDBAccess

Public Function GetEmployeeName(clockNo As Integer) As EmployeeName

    Dim employeeName As EmployeeName = Nothing
    Dim params As SqlParameter() = New SqlParameter() {New SqlParameter("@clockNo", clockNo)}

    Using tbl As DataTable = SMARTDBHelper.ExecuteParameterisedSelectCommand("getEmployeeName", CommandType.StoredProcedure, params)
        If tbl.Rows.Count = 1 Then
            Dim row As DataRow = tbl.Rows(0)
            employeeName = New EmployeeName
            employeeName.FirstName = row("INITIALS").ToString()
            employeeName.LastName = row("SURNAME").ToString()
        End If
    End Using
    Return employeeName
End Function
End Class



Public Class SMARTDBHelper
Const constr As String = "connection string goes here"


Friend Shared Function ExecuteParameterisedSelectCommand(cmdName As String, cmdType As CommandType, param As SqlParameter()) As DataTable

    Dim tbl As New DataTable()

    Using con As New SqlConnection(constr)
        Using cmd As SqlCommand = con.CreateCommand()
            cmd.CommandType = cmdType
            cmd.CommandText = cmdName
            cmd.Parameters.AddRange(param)

            Try
                If con.State <> ConnectionState.Open Then
                    con.Open()
                End If

                Using da As New SqlDataAdapter(cmd)
                    da.Fill(tbl)
                End Using
            Catch ex As Exception
                Throw
            End Try
        End Using
    End Using

    Return tbl

End Function

End Class

My BLL consists of the following class

Public Class EmployeeNameHandler

Private employee As EmployeeNameDBAccess

Public Sub New()
    employee = New EmployeeNameDBAccess()
End Sub

Public Function GetFirstname(clockNo As Integer) As String

    Dim emp = employee.GetEmployeeName(clockNo)
    Return emp.FirstName

End Function

Public Function GetLastname(clockNo As Integer) As String

    Dim emp = employee.GetEmployeeName(clockNo)
    Return emp.LastName

End Function
End Class

Then on my page I have the following code to populate the text boxes

Protected Sub txtCLock_TextChanged(sender As Object, e As EventArgs) Handles txtCLock.TextChanged

    Dim emp As New EmployeeNameHandler
    Dim cn As Integer = CInt(txtCLock.Text)
    Dim firstname = emp.GetFirstname(cn)
    Dim lastname = emp.GetLastname(cn)

    txtFirstName.Text = firstname
    txtLastName.Text = lastname

End Sub

I appreciate this is basic however I would be interested to gather opinion of if I am going down the correct route here? Could this be done in a better way? Also, there is next to no error handling here, this is a topic I'm very weak on so would appreciate any advice etc. Thanks

Answer 1

Just a few suggestions:

Probably advisable to use Int.TryParse instead of CInt(txtCLock.Text) for parsing user input so you don't get an invalid type exception.

On general error handling you might at least put a try/catch around the logic in txtCLock_TextChanged to catch any errors at a lower layer and either log them or present to the user in a nice way if applicable.

If you're not actually referencing the first/last name backing fields you can use automatic properties so you don't have to write a get/set for every property.