Click here to Skip to main content
15,892,537 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
is it possible for me to streamline these codes


if station1, update qty in stock_record table and insert bal into bincard table 
if station2, update qty1 in stock_record table and insert bal1 into bincard table 
if station3, update qty2 in stock_record table and insert bal2 into bincard table 

full code below
VB
If cboStation.Text = "STATION1" Then
            Dim strQuery13 As String
            conn.Open()
            strQuery13 = "UPDATE stock_record SET qty= '" & txtStkQty.Text & " ' where stkcode = '" & txtStkCode.Text & "'"
            recordinsert = New SqlCommand(strQuery13, conn)
            recordinsert.ExecuteNonQuery()
            conn.Close()

            Dim strQuery3 As String
            conn.Open()
            strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,cond,station,staff)Values('" & txtStkCode.Text & "','" & dtpInvDate.Value.ToString("yyyy/MM/dd") & "','0','" & txtQty.Text & "','" & txtStkQty.Text & "','SALES','" & cboStation.Text & "','" & frmMenu2.Label2.Text & "')"
            recordinsert = New SqlCommand(strQuery3, conn)
            recordinsert.ExecuteNonQuery()
            conn.Close()

        ElseIf cboStation.Text = "STATION2" Then
            Dim strQuery13 As String
            conn.Open()
            strQuery13 = "UPDATE stock_record SET qty1= '" & txtStkQty.Text & " ' where stkcode = '" & txtStkCode.Text & "'"
            recordinsert = New SqlCommand(strQuery13, conn)
            recordinsert.ExecuteNonQuery()
            conn.Close()

            Dim strQuery3 As String
            conn.Open()
            strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,bal1,cond,station,staff)Values('" & txtStkCode.Text & "','" & dtpInvDate.Value.ToString("yyyy/MM/dd") & "','0','" & txtQty.Text & "','0','" & txtStkQty.Text & "','SALES','" & cboStation.Text & "','" & frmMenu2.Label2.Text & "')"
            recordinsert = New SqlCommand(strQuery3, conn)
            recordinsert.ExecuteNonQuery()
            conn.Close()

        ElseIf cboStation.Text = "STATION3" Then
            Dim strQuery13 As String
            conn.Open()
            strQuery13 = "UPDATE stock_record SET qty2= '" & txtStkQty.Text & " ' where stkcode = '" & txtStkCode.Text & "'"
            recordinsert = New SqlCommand(strQuery13, conn)
            recordinsert.ExecuteNonQuery()
            conn.Close()

            Dim strQuery3 As String
            conn.Open()
            strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,bal2,cond,station,staff)Values('" & txtStkCode.Text & "','" & dtpInvDate.Value.ToString("yyyy/MM/dd") & "','0','" & txtQty.Text & "','0','" & txtStkQty.Text & "','SALES','" & cboStation.Text & "','" & frmMenu2.Label2.Text & "')"
            recordinsert = New SqlCommand(strQuery3, conn)
            recordinsert.ExecuteNonQuery()
            conn.Close()
        End If
Posted
Updated 18-Nov-13 0:53am
v4
Comments
phil.o 18-Nov-13 7:03am    
Not a question. What is the real problem?

1 solution

The phrase you are looking for is Code Refactoring[^]

Here is a walkthrough of how I looked at your code...

When you look at code like this try to see the similarities, or in this case, how few differences there are. Consider moving them so you don't have to keep retyping the same lines of code. For example step 1 would be to notice that the only differences are in strQuery13 and strQuery3 so tidy the code up to look like this
VB
Dim strQuery13 As String
Dim strQuery3 As String

        If cboStation.Text = "STATION1" Then
            strQuery13 = "UPDATE stock_record SET qty= '" & txtStkQty.Text & " ' where stkcode = '" & txtStkCode.Text & "'"
            strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,cond,station,staff)Values('" & txtStkCode.Text & "','" & dtpInvDate.Value.ToString("yyyy/MM/dd") & "','0','" & txtQty.Text & "','" & txtStkQty.Text & "','SALES','" & cboStation.Text & "','" & frmMenu2.Label2.Text & "')"

        ElseIf cboStation.Text = "STATION2" Then
            strQuery13 = "UPDATE stock_record SET qty1= '" & txtStkQty.Text & " ' where stkcode = '" & txtStkCode.Text & "'"
            strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,bal1,cond,station,staff)Values('" & txtStkCode.Text & "','" & dtpInvDate.Value.ToString("yyyy/MM/dd") & "','0','" & txtQty.Text & "','0','" & txtStkQty.Text & "','SALES','" & cboStation.Text & "','" & frmMenu2.Label2.Text & "')"

        ElseIf cboStation.Text = "STATION3" Then
            strQuery13 = "UPDATE stock_record SET qty2= '" & txtStkQty.Text & " ' where stkcode = '" & txtStkCode.Text & "'"
            strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,bal2,cond,station,staff)Values('" & txtStkCode.Text & "','" & dtpInvDate.Value.ToString("yyyy/MM/dd") & "','0','" & txtQty.Text & "','0','" & txtStkQty.Text & "','SALES','" & cboStation.Text & "','" & frmMenu2.Label2.Text & "')"
        End If

        conn.Open()
        recordinsert = New SqlCommand(strQuery13, conn)
        recordinsert.ExecuteNonQuery()
        conn.Close()

        conn.Open()
        recordinsert = New SqlCommand(strQuery3, conn)
        recordinsert.ExecuteNonQuery()
        conn.Close()

That makes it all a bit easier to read.

Now let's go back to those sql queries ... first thing to notice is that you have left yourself vulnerable to sql injection by inserting text directly into your queries. You should use sql parameters[^] instead. Here's the first query as an example
SQL
strQuery13 = "UPDATE stock_record SET qty= @StkQty where stkcode = @StkCode"
recordinsert.Parameters.Add(New SqlParameter("StkQty", txtStkQty.Text))
recordinsert.Parameters.Add(New SqlParameter("StkCode", txtStkCode.Text))

When you do all of them you will see even more things that are common between the stations.
In strQuery13 the only thing that is different is the name of the column that you are updating, so lets make that a parameter as well
recordinsert.Parameters.Add(New SqlParameter("colname", "qty"))
which means that strQuery13 can come out of the If statement-block and just be
strQuery13 = "UPDATE stock_record SET @colname= @StkQty where stkcode = @StkCode"
for all three cases.

When I look at strQuery3 I notice that all three versions of that are very similar, but for station 1 you just set column [bal], for Station 2 you set column [bal] = 0 and put a value in for [bal1] and for Station 3 you set column [bal]=0 and put a value in for [bal2].

This means that for Station 1 bal1 and bal2 will be NULL; for Station 2 bal2 will be NULL and for Station 3 bal1 will be NULL. This might cause you problems later when you are trying to read these values back. A better approach would be to always populate all three columns - either with the required value or with 0. So strQuery3 becomes
SQL
strQuery3 = "insert into bincard(stkcode,transdate,stkin,stkout,bal,bal1,bal2,cond,station,staff)"
strQuery3 += "Values('@StkCode,@InvDate,'0',@Qty,@bal,@bal1,@bal2,'SALES',@Station,@Label2)"

That made me go back and have a look at strQuery13 ... at first glance I thought I could do something similar with qty, qty1 and qty2, but then I realised that is an UPDATE statement so those columns could already have a value in them. Doing the same thing would therefore NOT be a good idea.

This is the point of this exercise, don't try to do everything at once, but do go back and have another look. The trick is to know when is a good time to stop. Don't lose clarity in your code for the sake of saving a couple of lines unless you need cutting-edge performance (and possibly not even then!)

This got me to this stage ...
VB
Dim bal As Double = 0   'balance for Station 1 - Must be iniitialised to 0
Dim bal1 As Double = 0  'balance for Station 2 - Must be iniitialised to 0
Dim bal2 As Double = 0  'balance for Station 3 - Must be iniitialised to 0
Dim colname As String

If cboStation.Text = "STATION1" Then
    colname = "qty"
    bal = txtStkQty.Text
ElseIf cboStation.Text = "STATION2" Then
    colname = "qty1"
    bal1 = txtStkQty.Text
ElseIf cboStation.Text = "STATION3" Then
    colname = "qty2"
    bal2 = txtStkQty.Text
End If

'Query13
Dim strQuery13 As String = "UPDATE stock_record SET @colname= @StkQty where stkcode = @StkCode"
recordinsert = New SqlCommand(strQuery13, conn)

recordinsert.Parameters.Add(New SqlParameter("StkQty", txtStkQty.Text))
recordinsert.Parameters.Add(New SqlParameter("colname", colname))
recordinsert.Parameters.Add(New SqlParameter("StkCode", txtStkCode.Text))

conn.Open()
recordinsert.ExecuteNonQuery()
conn.Close()

'Query3
Dim strQuery3 As String = "insert into bincard(stkcode,transdate,stkin,stkout,bal,bal1,bal2,cond,station,staff)"
strQuery3 += "Values('@StkCode,@InvDate,'0',@Qty,@bal,@bal1,@bal2,'SALES',@Station,@Label2)"
recordinsert = New SqlCommand(strQuery3, conn)

recordinsert.Parameters.Add(New SqlParameter("StkCode", txtStkCode.Text))
recordinsert.Parameters.Add(New SqlParameter("InvDate", dtpInvDate.Value.ToString("yyyy/MM/dd"))
recordinsert.Parameters.Add(New SqlParameter("Station", cboStation.Text))
recordinsert.Parameters.Add(New SqlParameter("Label", frmMenu2.Label2.Text))
recordinsert.Parameters.Add(New SqlParameter("Qty", txtQty.Text))
recordinsert.Parameters.Add(New SqlParameter("Bal", bal))
recordinsert.Parameters.Add(New SqlParameter("Bal1", bal1))
recordinsert.Parameters.Add(New SqlParameter("Bal2", bal2))

conn.Open()
recordinsert.ExecuteNonQuery()
conn.Close()


Now this won't even compile ... I have some issues over balances and quantities being set up as strings in your original sql for example. And there may be the odd spelling mistake or two in here but hopefully I've set you off in the right direction
 
Share this answer
 
Comments
kabifarm 19-Nov-13 8:02am    
Thanks very much CHill6024K for ur quick response. am very grateful for painstakingly going through the code and refactoring them for me. these will help me to refactor most of my othe codes.

but there is this statement of urs that i wish to understand more: "you have left yourself vulnerable to sql injection by inserting text directly into your queries.". what does that mean so that i can know how to avoid same subsequently

Also, i ve tried the first set of codes in my form and the worked perfectly. but for the second set with parameters am still twieaking
CHill60 19-Nov-13 9:00am    
If you go to the link on SQL Parameters, right at the bottom of that article is some information and links about SQL injection, how it works and how to avoid it.
There is even a joke cartoon about it ... http://xkcd.com/327/[^]

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900