Background: I'm designing a system (VB/WinForms) that uses a database(MS SQL Server 2008 R2) to track people, their stock account #'s, which stocks they are investing in, and the payout of those stocks.

Basically, just reading/writing to the database and some time-elapsed functionality. (One payout per year, etc.) I have forms where users are entering employees to the database, adding accounts, etc.

My issue: I never truly understood OOP, however I am getting more involved with it now, and while my program works, I want the code to be better (look better, be more flexible, utilize classes/objects more, etc.)

That being said, how can I make this code more Objected-Oriented?

Note: I am also utilizing ReSharper, however that can only do so much and I don't want to have to rely on a tool for the rest of my career.

Note: This project utilizes an encryption class.

Imports System.Data.SqlClient
Public Class EmployeeUpdateFrm
    Private _id As String
    Dim _seqId As Integer

Private Sub Load(sender As Object, e As EventArgs) Handles MyBase.Load

    Dim conn1 As New SqlConnection("Data Source=SQLTEST_HR,4000\SQLEXPRESS;Integrated Security=True")


    Dim ds1 As New DataSet()
    Using da As New SqlDataAdapter(qry, conn1)
        'fill data set 1 for combobox
    End Using

    With CompanyCbx
        'what the user sees
        .DisplayMember = "CMPNY_NM"
        'value behind each display member
        .ValueMember = "CMPNY_SEQ_ID"
        .DataSource = ds1.Tables(0)
        .SelectedIndex = 0
    End With
    'close connection
    Dim index As Integer = 1
    'The vertical spacing between rows of controls relative to the textboxes.
    Dim yMargin As Integer = 10
    Dim query As String
    'Create a new instance of the encryption class.
    Dim strKey As String = "Key1"
    Dim Crypto As ClsCrypt
    Crypto = New ClsCrypt(strKey)

    Dim eID As String = Crypto.EncryptData(_id)

    query &= "                          EMPL_FILE_NO, EMPL_SPRTN_DT, CMPNY_SEQ_ID, EMPL_ACTV_IND, BEG_DT, END_DT "
    query &= "  FROM EMPL"
    query &= "  WHERE EMPL_ID = @ID; "

    'New DataSet object to hold employee records.
    Using ds As New DataSet()
        Using conn As New SqlConnection("Data Source=SQLTEST_HR,4000\SQLEXPRESS;Integrated Security=True")
            Using da As New SqlDataAdapter()
                da.SelectCommand = New SqlCommand(query, conn)
                da.SelectCommand.Parameters.Add(New SqlParameter("@ID", eID))
            End Using
        End Using

        If CompanyCbx.Items.Count > 0 Then
            CompanyCbx.SelectedIndex = ds.Tables(0).Rows(0).Item(15).ToString
        End If

            For i As Integer = 0 To ds.Tables(0).Rows.Count - 1
                For z As Integer = 0 To ds.Tables(0).Columns.Count - 1
                    If z <> 0 Then
                        'Decrypt all rows and columns
                        ds.Tables(0).Rows(i)(z) = Crypto.DecryptData(ds.Tables(0).Rows(i)(z))
                    End If
        Catch ex As Exception
        End Try

I also ask (without being opinionated), with programs like this (fairly small, read/write to database with no extraordinary functionality) how flexible is the coding? By that I mean, I don't really see how I could use something like:

Dim Person as New Person()

Class Person
Public firstName as String
Public lastName as String


Especially because I utilize DataSets so heavily.


If you want to learn how to use classes, the best approach is to use them while building a small program. I suggest you do create a Person class and that you stop using dataset. This will be a great learnign experience.

Using numbers instead of const or strings can create a mantenance problem. At a glance, it's not easy to know what 15 mean.


Remove all the database query from the UI. It'll also make the code more readable and you'll be able to reuse the functions if needed. You should have the connection string at one place.

Public Class EmployeeUpdateFrm

    Private Sub Load(sender As Object, e As EventArgs) Handles MyBase.Load


        ' ...

    End Sub

    Sub LoadDropDown()

        Dim companies As List(Of Company)

        companies = DAL.GetCompanies()

        With CompanyCbx
            'what the user sees
            .DisplayMember = "Name"
            'value behind each display member
            .ValueMember = "Id"
            .DataSource = companies
            .SelectedIndex = 0
        End With

    End Sub

End Class

Public Class DAL

    Private Shared _connectionString As String = "Data Source=SQLTEST_HR,4000\SQLEXPRESS;Integrated Security=True"

    Public Shared Function GetCompanies() As List(Of Company)
       ' ...
    End Function

End Class

Public Class Company

    Public Property Id As Integer
    Public Property Name As String

End Class
  • \$\begingroup\$ Yes, this is exactly the type of answer I was looking for. Please understand that when you say "stop using datasets" I do not know what else to use. What you're seeing is the only way I have been able to do this \$\endgroup\$ – Mark Apr 7 '14 at 14:44
  • \$\begingroup\$ @MarkLaREZZA Note that there is nothing wrong in using dataset. But you have the ability to slowly change each dataset to a class and learn along the way how to use them. In the small example I gave, instead of using the dataset ds1 I used a List(Of Company) \$\endgroup\$ – the_lotus Apr 7 '14 at 14:56
  • \$\begingroup\$ Ah, okay. I see it a little bit clearer now. I agree 100% that the dataset index usage is unreadable and unclear. That's what truly prompted me to want to learn how to code better. Thanks! \$\endgroup\$ – Mark Apr 7 '14 at 15:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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