HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

System for tracking stock information

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
trackingsystemstockforinformation

Problem

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 qry As String = "SELECT CMPNY_SEQ_ID, CMPNY_NM FROM CMPNY"

Dim ds1 As New DataSet()
Using da As New SqlDataAdapter(qry, conn1)
'fill data set 1 for combobox
da.Fill(ds1)
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
conn1.Close()
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 Strin

Solution

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.

ds.Tables(0).Rows(0).Item(15)


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

        LoadDropDown()

        ' ...

    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

Code Snippets

ds.Tables(0).Rows(0).Item(15)
Public Class EmployeeUpdateFrm

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

        LoadDropDown()

        ' ...

    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

Context

StackExchange Code Review Q#45527, answer score: 2

Revisions (0)

No revisions yet.