Question The right way....

r3plica

Well-known member
Joined
Mar 4, 2010
Messages
86
Programming Experience
3-5
I use collections.
I have been for about 2 years and they have worked fine for me.
I decided that I should read about design patterns and UML to improve my programming.
When reading these books, I found that perhaps my collects were not as they were supposed to be.
My classes all have one thing in common, they contain properties and nothing else.
Here is an example of one of them

VB.NET:
Public Class User

        Private _UserId As String
        Public Property UserId() As String
            Get
                Return _UserId
            End Get
            Set(ByVal value As String)
                _UserId = value
            End Set
        End Property

        Private _ClientId As String
        Public Property ClientId() As String
            Get
                Return _ClientId
            End Get
            Set(ByVal value As String)
                _ClientId = value
            End Set
        End Property

        Private _ClientName As String
        Public Property ClientName() As String
            Get
                Return _ClientName
            End Get
            Set(ByVal value As String)
                _ClientName = value
            End Set
        End Property

        Private _ClientDescription As String
        Public Property ClientDescription() As String
            Get
                Return _ClientDescription
            End Get
            Set(ByVal value As String)
                _ClientDescription = value
            End Set
        End Property

        Private _ClientLogo As String
        Public Property ClientLogo() As String
            Get
                Return _ClientLogo
            End Get
            Set(ByVal value As String)
                _ClientLogo = value
            End Set
        End Property

        Private _UserName As String
        Public Property UserName() As String
            Get
                Return _UserName
            End Get
            Set(ByVal value As String)
                _UserName = value
            End Set
        End Property

        Private _Role As String
        Public Property Role() As String
            Get
                Return _Role
            End Get
            Set(ByVal value As String)
                _Role = value
            End Set
        End Property

        Private _RoleId As String
        Public Property RoleId() As String
            Get
                Return _RoleId
            End Get
            Set(ByVal value As String)
                _RoleId = value
            End Set
        End Property

        Private _CompanyId As String
        Public Property CompanyId() As String
            Get
                Return _CompanyId
            End Get
            Set(ByVal value As String)
                _CompanyId = value
            End Set
        End Property

        Private _companyName As String
        Public Property CompanyName() As String
            Get
                Return _companyName
            End Get
            Set(ByVal value As String)
                _companyName = value
            End Set
        End Property

        Private _CompanyDescription As String
        Public Property CompanyDescription() As String
            Get
                Return _CompanyDescription
            End Get
            Set(ByVal value As String)
                _CompanyDescription = value
            End Set
        End Property

        Private _CompanyLogo As String
        Public Property CompanyLogo() As String
            Get
                Return _CompanyLogo
            End Get
            Set(ByVal value As String)
                _CompanyLogo = value
            End Set
        End Property

        Private _Email As String
        Public Property Email() As String
            Get
                Return _Email
            End Get
            Set(ByVal value As String)
                _Email = value
            End Set
        End Property

        Private _sendLimit As Integer
        Public Property SendLimit() As Integer
            Get
                Return _sendLimit
            End Get
            Set(ByVal value As Integer)
                _sendLimit = value
            End Set
        End Property


        Private _CreateDate As DateTime
        Public Property CreateDate() As DateTime
            Get
                Return _CreateDate
            End Get
            Set(ByVal value As DateTime)
                _CreateDate = value
            End Set
        End Property

        Private _lastLoginDate As DateTime
        Public Property LastLoginDate() As DateTime
            Get
                Return _lastLoginDate
            End Get
            Set(ByVal value As DateTime)
                _lastLoginDate = value
            End Set
        End Property

        Private _lastLockoutDate As DateTime
        Public Property LastLockoutDate() As DateTime
            Get
                Return _lastLockoutDate
            End Get
            Set(ByVal value As DateTime)
                _lastLockoutDate = value
            End Set
        End Property

        Private _isLockedOut As Boolean
        Public Property IsLockedOut() As Boolean
            Get
                Return _isLockedOut
            End Get
            Set(ByVal value As Boolean)
                _isLockedOut = value
            End Set
        End Property

        Private _isApproved As Boolean
        Public Property IsApproved() As Boolean
            Get
                Return _isApproved
            End Get
            Set(ByVal value As Boolean)
                _isApproved = value
            End Set
        End Property
End Class

Then I would use a business layer and Data layer to create/edit/delete and retrieve from the database.

VB.NET:
Imports System.Data.SqlClient
Imports System.Configuration

Imports BD.Common

Namespace Data

    Public Class ProfileData
        Inherits BaseDataAccess

        Public Shared Sub CreateUser(ByVal pUser As User)
            Dim SQLCmd As New SqlCommand
            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "CreateUser"
            SQLCmd.Parameters.Add("UserId", SqlDbType.UniqueIdentifier).Value = New Guid(pUser.UserId)
            SQLCmd.Parameters.Add("ClientId", SqlDbType.UniqueIdentifier).Value = New Guid(pUser.ClientId)
            SQLCmd.Parameters.Add("CompanyId", SqlDbType.UniqueIdentifier).Value = New Guid(pUser.CompanyId)
            SQLCmd.Parameters.Add("CompanyName", SqlDbType.VarChar, 100).Value = pUser.CompanyName
            SQLCmd.Parameters.Add("Description", SqlDbType.VarChar, 255).Value = pUser.CompanyDescription
            SQLCmd.Parameters.Add("Logo", SqlDbType.VarChar, 100).Value = pUser.CompanyLogo
            SQLCmd.Parameters.Add("SendLimit", SqlDbType.Int).Value = pUser.SendLimit
            ExecuteNonSelect(SQLCmd)
        End Sub

        Public Shared Sub UpdateSettings(ByVal pUser As User, ByVal sID As String)
            Dim SQLCmd As New SqlCommand
            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "UpdateSettings"
            SQLCmd.Parameters.Add("ID", SqlDbType.UniqueIdentifier).Value = New Guid(sID)
            SQLCmd.Parameters.Add("CompanyName", SqlDbType.VarChar, 100).Value = pUser.CompanyName
            SQLCmd.Parameters.Add("Description", SqlDbType.VarChar, 255).Value = pUser.CompanyDescription
            SQLCmd.Parameters.Add("Logo", SqlDbType.VarChar, 100).Value = pUser.CompanyLogo
            ExecuteNonSelect(SQLCmd)
        End Sub

        Public Shared Function GetProfile(ByVal UserId As String) As User
            Dim SQLCmd As New SqlCommand
            Dim ds As New DataSet

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "GetProfile"
            SQLCmd.Parameters.Add("UserId", SqlDbType.UniqueIdentifier).Value = New Guid(UserId)
            ds = FillDataSet(SQLCmd)
            Return ParseUser(ds.Tables(0).Rows(0))
        End Function

        Public Shared Function GetProfilesByClient(ByVal ClientId As String, ByVal RoleName As String) As UserCollection
            Dim cUser As New UserCollection
            Dim SQLCmd As New SqlCommand
            Dim ds As New DataSet

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "GetProfilesByClient"
            SQLCmd.Parameters.Add("ClientId", SqlDbType.UniqueIdentifier).Value = New Guid(ClientId)
            SQLCmd.Parameters.Add("Role", SqlDbType.VarChar, 100).Value = RoleName
            ds = FillDataSet(SQLCmd)
            For Each dr As DataRow In ds.Tables(0).Rows
                cUser.Add(ParseUser(dr))
            Next
            Return cUser
        End Function

        Public Shared Function GetProfiles() As UserCollection
            Dim cUser As New UserCollection
            Dim SQLCmd As New SqlCommand
            Dim ds As New DataSet

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "GetProfiles"
            ds = FillDataSet(SQLCmd)
            For Each dr As DataRow In ds.Tables(0).Rows
                cUser.Add(ParseUser(dr))
            Next
            Return cUser
        End Function

        Public Shared Sub DeleteProfile(ByVal UserId As String)
            Dim SQLCmd As New SqlCommand

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "DeleteProfile"
            SQLCmd.Parameters.Add("UserId", SqlDbType.UniqueIdentifier).Value = New Guid(UserId)
            ExecuteNonSelect(SQLCmd)
        End Sub

        Private Shared Function ParseUser(ByVal dr As DataRow) As User
            Dim cUser As New User
            With cUser
                .UserId = Trim(dr("UserId"))
                .ClientId = Trim(dr("ClientId"))
                .ClientName = Trim(dr("ClientName"))
                .ClientDescription = Trim(dr("ClientDescription"))
                .ClientLogo = Trim(dr("ClientLogo"))
                .UserName = Trim(dr("UserName"))
                .RoleId = Trim(dr("RoleId"))
                .Role = Trim(dr("Role"))
                .CompanyId = Trim(dr("CompanyId"))
                .CompanyName = Trim(dr("CompanyName"))
                .CompanyDescription = Trim(dr("CompanyDescription"))
                .CompanyLogo = Trim(dr("CompanyLogo"))
                .Email = Trim(dr("Email"))
                .CreateDate = CDate(dr("CreateDate"))
                .LastLoginDate = CDate(dr("LastLoginDate"))
                .LastLockoutDate = CDate(dr("LastLockoutDate"))
                .IsLockedOut = CBool(dr("IsLockedOut"))
                .IsApproved = CBool(dr("IsApproved"))
                .SendLimit = CInt(dr("SendLimit"))
            End With
            Return cUser
        End Function
    End Class

End Namespace

After reading my books, I decided that perhaps my classes could be better.
I though that they should include methods themselves, so I modified my user class and created this:

VB.NET:
Imports System.Web.Security
Imports System.Web
Imports System.Data.SqlClient
Imports BD.Data.ProfileData

Namespace Common
    Public Class User
        Public Sub New()
            ' Default constructor
        End Sub

        Public Sub New(ByVal dr As sqlDataReader)
            ParseUser(dr)
        End Sub

        Public Sub New(ByVal dr As DataRow)
            ParseUser(dr)
        End Sub

        Public Sub New(ByVal UserId As String)
            ParseUser(GetProfile(UserId))
        End Sub

        Private _UserId As String
        Public Property UserId() As String
            Get
                Return _UserId
            End Get
            Set(ByVal value As String)
                _UserId = value
            End Set
        End Property

        Private _ClientId As String
        Public Property ClientId() As String
            Get
                Return _ClientId
            End Get
            Set(ByVal value As String)
                _ClientId = value
            End Set
        End Property

        Private _ClientName As String
        Public Property ClientName() As String
            Get
                Return _ClientName
            End Get
            Set(ByVal value As String)
                _ClientName = value
            End Set
        End Property

        Private _ClientDescription As String
        Public Property ClientDescription() As String
            Get
                Return _ClientDescription
            End Get
            Set(ByVal value As String)
                _ClientDescription = value
            End Set
        End Property

        Private _ClientLogo As String
        Public Property ClientLogo() As String
            Get
                Return _ClientLogo
            End Get
            Set(ByVal value As String)
                _ClientLogo = value
            End Set
        End Property

        Private _UserName As String
        Public Property UserName() As String
            Get
                Return _UserName
            End Get
            Set(ByVal value As String)
                _UserName = value
            End Set
        End Property

        Private _Role As String
        Public Property Role() As String
            Get
                Return _Role
            End Get
            Set(ByVal value As String)
                _Role = value
            End Set
        End Property

        Private _RoleId As String
        Public Property RoleId() As String
            Get
                Return _RoleId
            End Get
            Set(ByVal value As String)
                _RoleId = value
            End Set
        End Property

        Private _CompanyId As String
        Public Property CompanyId() As String
            Get
                Return _CompanyId
            End Get
            Set(ByVal value As String)
                _CompanyId = value
            End Set
        End Property

        Private _companyName As String
        Public Property CompanyName() As String
            Get
                Return _companyName
            End Get
            Set(ByVal value As String)
                _companyName = value
            End Set
        End Property

        Private _CompanyDescription As String
        Public Property CompanyDescription() As String
            Get
                Return _CompanyDescription
            End Get
            Set(ByVal value As String)
                _CompanyDescription = value
            End Set
        End Property

        Private _CompanyLogo As String
        Public Property CompanyLogo() As String
            Get
                Return _CompanyLogo
            End Get
            Set(ByVal value As String)
                _CompanyLogo = value
            End Set
        End Property

        Private _Email As String
        Public Property Email() As String
            Get
                Return _Email
            End Get
            Set(ByVal value As String)
                _Email = value
            End Set
        End Property

        Private _sendLimit As Integer
        Public Property SendLimit() As Integer
            Get
                Return _sendLimit
            End Get
            Set(ByVal value As Integer)
                _sendLimit = value
            End Set
        End Property


        Private _CreateDate As DateTime
        Public Property CreateDate() As DateTime
            Get
                Return _CreateDate
            End Get
            Set(ByVal value As DateTime)
                _CreateDate = value
            End Set
        End Property

        Private _lastLoginDate As DateTime
        Public Property LastLoginDate() As DateTime
            Get
                Return _lastLoginDate
            End Get
            Set(ByVal value As DateTime)
                _lastLoginDate = value
            End Set
        End Property

        Private _lastLockoutDate As DateTime
        Public Property LastLockoutDate() As DateTime
            Get
                Return _lastLockoutDate
            End Get
            Set(ByVal value As DateTime)
                _lastLockoutDate = value
            End Set
        End Property

        Private _isLockedOut As Boolean
        Public Property IsLockedOut() As Boolean
            Get
                Return _isLockedOut
            End Get
            Set(ByVal value As Boolean)
                _isLockedOut = value
            End Set
        End Property

        Private _isApproved As Boolean
        Public Property IsApproved() As Boolean
            Get
                Return _isApproved
            End Get
            Set(ByVal value As Boolean)
                _isApproved = value
            End Set
        End Property

        Public Function updateUser() As Boolean
            Try
                Dim oUser As MembershipUser = Membership.GetUser(Me.UserName) ' Get the user
                oUser.IsApproved = Me.IsApproved ' Set the approved status of the user
                Membership.UpdateUser(oUser) ' Perform an update, the changes won't be made if this is not called
                If Not Me.IsLockedOut Then oUser.UnlockUser() ' Unlock the user if we have unchecked the checkbox
                Membership.UpdateUser(oUser) ' I could be wrong, but it seems you must update the user after every command
                Return True
            Catch ex As Exception
                Return False
            End Try
        End Function

        Public Sub save()
            ' TODO: Save
            HttpContext.Current.Session.Remove("Profile") ' Remove our session before saving or creating our profile (This is so it gets recreated in the basepage with the new saved settings).
            CreateUser(Me)
        End Sub

        Public Function saveCompanyDetails() As Boolean
            Try
                UpdateSettings(Me, Me.CompanyId)
                Return True
            Catch ex As Exception
                Return False
            End Try
        End Function

        Public Function saveUserDetails() As Boolean
            Try
                UpdateSettings(Me, Me.UserId)
                Return True
            Catch ex As Exception
                Return False
            End Try
        End Function

        Private Sub ParseUser(ByVal dr As SqlDataReader)
            With Me
                .UserId = Trim(dr("UserId"))
                .ClientId = Trim(dr("ClientId"))
                .ClientName = Trim(dr("ClientName"))
                .ClientDescription = Trim(dr("ClientDescription"))
                .ClientLogo = Trim(dr("ClientLogo"))
                .UserName = Trim(dr("UserName"))
                .RoleId = Trim(dr("RoleId"))
                .Role = Trim(dr("Role"))
                .CompanyId = Trim(dr("CompanyId"))
                .CompanyName = Trim(dr("CompanyName"))
                .CompanyDescription = Trim(dr("CompanyDescription"))
                .CompanyLogo = Trim(dr("CompanyLogo"))
                .Email = Trim(dr("Email"))
                .CreateDate = CDate(dr("CreateDate"))
                .LastLoginDate = CDate(dr("LastLoginDate"))
                .LastLockoutDate = CDate(dr("LastLockoutDate"))
                .IsLockedOut = CBool(dr("IsLockedOut"))
                .IsApproved = CBool(dr("IsApproved"))
                .SendLimit = CInt(dr("SendLimit"))
            End With
        End Sub

        Private Sub ParseUser(ByVal dr As DataRow)
            With Me
                .UserId = Trim(dr("UserId"))
                .ClientId = Trim(dr("ClientId"))
                .ClientName = Trim(dr("ClientName"))
                .ClientDescription = Trim(dr("ClientDescription"))
                .ClientLogo = Trim(dr("ClientLogo"))
                .UserName = Trim(dr("UserName"))
                .RoleId = Trim(dr("RoleId"))
                .Role = Trim(dr("Role"))
                .CompanyId = Trim(dr("CompanyId"))
                .CompanyName = Trim(dr("CompanyName"))
                .CompanyDescription = Trim(dr("CompanyDescription"))
                .CompanyLogo = Trim(dr("CompanyLogo"))
                .Email = Trim(dr("Email"))
                .CreateDate = CDate(dr("CreateDate"))
                .LastLoginDate = CDate(dr("LastLoginDate"))
                .LastLockoutDate = CDate(dr("LastLockoutDate"))
                .IsLockedOut = CBool(dr("IsLockedOut"))
                .IsApproved = CBool(dr("IsApproved"))
                .SendLimit = CInt(dr("SendLimit"))
            End With
        End Sub
    End Class

End Namespace

and the Data Layer was changed to this:

VB.NET:
Imports System.Data.SqlClient
Imports System.Configuration

Imports BD.Common

Namespace Data

    Public Class ProfileData
        Inherits BaseDataAccess

        Public Shared Sub CreateUser(ByVal pUser As User)
            Dim SQLCmd As New SqlCommand
            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "CreateUser"
            SQLCmd.Parameters.Add("UserId", SqlDbType.UniqueIdentifier).Value = New Guid(pUser.UserId)
            SQLCmd.Parameters.Add("ClientId", SqlDbType.UniqueIdentifier).Value = New Guid(pUser.ClientId)
            SQLCmd.Parameters.Add("CompanyId", SqlDbType.UniqueIdentifier).Value = New Guid(pUser.CompanyId)
            SQLCmd.Parameters.Add("CompanyName", SqlDbType.VarChar, 100).Value = pUser.CompanyName
            SQLCmd.Parameters.Add("Description", SqlDbType.VarChar, 255).Value = pUser.CompanyDescription
            SQLCmd.Parameters.Add("Logo", SqlDbType.VarChar, 100).Value = pUser.CompanyLogo
            SQLCmd.Parameters.Add("SendLimit", SqlDbType.Int).Value = pUser.SendLimit
            ExecuteNonSelect(SQLCmd)
        End Sub

        Public Shared Sub UpdateSettings(ByVal pUser As User, ByVal sID As String)
            Dim SQLCmd As New SqlCommand
            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "UpdateSettings"
            SQLCmd.Parameters.Add("ID", SqlDbType.UniqueIdentifier).Value = New Guid(sID)
            SQLCmd.Parameters.Add("CompanyName", SqlDbType.VarChar, 100).Value = pUser.CompanyName
            SQLCmd.Parameters.Add("Description", SqlDbType.VarChar, 255).Value = pUser.CompanyDescription
            SQLCmd.Parameters.Add("Logo", SqlDbType.VarChar, 100).Value = pUser.CompanyLogo
            ExecuteNonSelect(SQLCmd)
        End Sub

        Public Shared Function GetProfile(ByVal UserId As String) As DataRow
            Dim SQLCmd As New SqlCommand
            Dim ds As New DataSet

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "GetProfile"
            SQLCmd.Parameters.Add("UserId", SqlDbType.UniqueIdentifier).Value = New Guid(UserId)
            ds = FillDataSet(SQLCmd)
            Return ds.Tables(0).Rows(0)
        End Function

        Public Shared Function GetProfilesByClient(ByVal ClientId As String, ByVal RoleName As String) As UserCollection
            Dim cUser As New UserCollection
            Dim SQLCmd As New SqlCommand

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "GetProfilesByClient"
            SQLCmd.Parameters.Add("ClientId", SqlDbType.UniqueIdentifier).Value = New Guid(ClientId)
            SQLCmd.Parameters.Add("Role", SqlDbType.VarChar, 100).Value = RoleName
            OpenConnection(SQLCmd)
            Dim dr As SqlDataReader = SQLCmd.ExecuteReader
            While dr.Read
                cUser.Add(New User(dr))
            End While
            dr.Close()
            CloseConnection()
            Return cUser
        End Function

        Public Shared Function GetProfiles() As UserCollection
            Dim cUser As New UserCollection
            Dim SQLCmd As New SqlCommand

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "GetProfiles"
            OpenConnection(SQLCmd)
            Dim dr As SqlDataReader = SQLCmd.ExecuteReader
            While dr.Read
                cUser.Add(New User(dr))
            End While
            dr.Close()
            CloseConnection()
            Return cUser
        End Function

        Public Shared Sub DeleteProfile(ByVal UserId As String)
            Dim SQLCmd As New SqlCommand

            SQLCmd.CommandType = CommandType.StoredProcedure
            SQLCmd.CommandText = "DeleteProfile"
            SQLCmd.Parameters.Add("UserId", SqlDbType.UniqueIdentifier).Value = New Guid(UserId)
            ExecuteNonSelect(SQLCmd)
        End Sub
    End Class

End Namespace

Now you will notice I have used a data reader for the collections, but for the single object I have used a dataset. This is because I couldnt find a way to return the results without creating a new User.
As far as I can tell, this is better than the way I was doing it before.

BUT.

I have now created strong coupling on my pages, because in order to get a user, I have to do this:

VB.NET:
New User(Membership.GetUser.ProviderUserKey.ToString)

So, have I just wasted my time?
Should I create a new layer?
I mean all this has removed my previously mentioned Business Layer.... Should I create functions in there that look like this:

VB.NET:
        Public Shared Function returnProfile(ByVal UserId As String) As User
            Return New User(UserId)
        End Function

and then on my page have this:

VB.NET:
dim myUser as User = returnProfile(Membership.GetUser.ProviderUserKey.ToString)

that just means that the strong coupling is in the business layer and not the client layer.

Can you see why I am getting confused? :D

Any help will be greatly appreciated. I know there is a lot to read and I am sorry for that.
 
no one? :(
 
You should not have a dependency on a specific data access technology in your business objects because you should be able to change your data access technology with no change to those business objects. There should be another layer whose responsibility it is to map between the data access layer and the business logic layer. If the data access technology changes then that mapping layer changes but the business logic remains exactly as it was.
 
how many layers would you have?
I already have:

Business Layer - which has all of my logic in it (well most of it)
Common Layer - which houses all of my objects (classes and collections)
Data Layer - which handles connecting to my database

Are you saying I should have another layer (what would I call it?) that the business layer calls methods from and that layer then decides where to get the data from?
I am a bit lost on how I would lay that out. Isn't the Data Access Layer the part that decides where to get the data from?
I know at the moment the classes are all inherited from by BaseDataAcccess.vb but that is because they all use SQL :)
If I had anything that accessed XML or MySQL then I would probably created another Base class and inherit from that (if neccessary).

My questions above are more related to best programming practices.
I read that having all classes uncoupled was a good practice but ultimately unrealistic. You should aim for uncoupled classes, but if you application is standalone and will never have other Clients interacting with it, then it is up to you.
I was getting really confused because I use my Common classes as objects that represent ceratin blocks of information. Each of the common classes just contained Properties that were parsed from rows returned from my stored procedures.
Now it looks like classes are more than that. I should put some logic in there that relates to the common class. But I should not allow a common class to create instances of itself, nor can it delete itself either.....which means the business layer is needed.

I have been trying to learn when to use design patterns and trying to learn UML. Because of this I have seen lots of things that seem totally different from the way that I program and it is making me ask: Am I doing it wrong?
Which is a question I am sure all programmers ask, but still. I need guidance. I am a single developer here, so I have no one I can ask :)
 
Back
Top