0

i have this code on my ADD button, is there's other way to shorten this statement of code.

Private Sub btnAdd_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles btnAdd.Click

    On Error GoTo ErrSQL

    Dim cmd As New OleDb.OleDbCommand
    If Not cnn.State = ConnectionState.Open Then
        'Open connection if it is not yet open
        cnn.Open()
    End If
    cmd.Connection = cnn
    'check whether add new update
    If Me.txtstdID.Tag & "" = "" Then
        'add new
        'add data to table
        cmd.CommandText = "INSERT INTO inventory (ID,BRAND,SPECIFICATION,STATUS) Values ('" & Me.txtstdID.Text & "','" & Me.cboBrand.Text & "','" & Me.txtDescription.Text & "','" & strStat & "')"
        cmd.ExecuteNonQuery()


    Else
        'update data in table
        cmd.CommandText = "UPDATE inventory  SET ID =" & Me.txtstdID.Text & ",BRAND='" & Me.cboBrand.Text & "', SPECIFICATION='" & Me.txtDescription.Text & "', STATUS = '" & strStat & "', WHERE ID=" & Me.txtstdID.Text & ""
        cmd.ExecuteNonQuery()
    End If
    'refresh data in list
    RefreshData()
    'clear form
    Me.btnClear.PerformClick()

    'close connection
    cnn.Close()


    Exit Sub
 ErrSQL:
    MsgBox(Err.Description)


End Sub
Steve
  • 213,761
  • 22
  • 232
  • 286
beginnerVB.Net
  • 43
  • 1
  • 2
  • 9
  • 1
    Don't worry about shortening the code, worry about [Sql Injection](http://stackoverflow.com/questions/332365/xkcd-sql-injection-please-explain) – Steve Mar 01 '13 at 09:36

1 Answers1

3

There isn't too much to do to shorten the code, there is a lot to do to prevent Sql Injection and parsing problems. I will try to change your code to this

Try
    Using cnn = new OleDbConnection(constring)
        Dim cmd As New OleDb.OleDbCommand
        cnn.Open()
        cmd.Connection = cnn
        Dim cmdText as String

        'check whether add new update
        If Me.txtstdID.Tag & "" = "" Then
              cmdText = "INSERT INTO inventory (ID,BRAND,SPECIFICATION,STATUS) " + 
                         "Values (@ID, @Brand, @specs, @stat)"
        else
              cmdText = "UPDATE inventory SET ID=@ID, BRAND=@Brand,SPECIFICATION=@specs" +
                        "STATUS = @stat WHERE ID=@ID"    
        End If
        cmd.CommandText = cmdText
        cmd.Parametes.AddWithValue("@ID", Me.txtstdID.Text)
        cmd.Parametes.AddWithValue("@Brand", Me.cboBrand.Text)
        cmd.Parametes.AddWithValue("@specs", Me.txtDescription.Text)
        cmd.Parametes.AddWithValue("@stat", strStat)
        cmd.ExecuteNonQuery()
   End Using
   'refresh data in list
   RefreshData()
   'clear form
   Me.btnClear.PerformClick()
Catch(x As Exception)
    MsgBox(x.Message)
End Try

I was tempted to remove the redundant SET ID=@ID in the Update statement, but, then you have to add the @ID parameter after the other parameters because in OleDb the parameter order matters

Steve
  • 213,761
  • 22
  • 232
  • 286
  • I think you could remove the SET ID=@ID by moving the AddWithValue for the ID into the if block. – Thierry Mar 01 '13 at 17:59
  • No, because, as I have said, in OleDb the orders of parameters matters. Without that ID in first position, we need to introduce a different sequence in the AddWithValue because the ID is not in the same position between INSERT and UPDATE commands – Steve Mar 01 '13 at 21:56
  • sir, Steve. this is the error i encountered 1. "costring' is not declared. IT may be inaccessible due to its protection level 2.strStat' is not declared. It may be inaccesssible due to its protection level 3.x As exception is not declared. It may be Inaccessible due to its protection level – beginnerVB.Net Mar 02 '13 at 02:30
  • Ah. I see now what I missed. – Thierry Mar 05 '13 at 18:28