0

I am using Visual Studio 2019 community version and I have a form I connected to SQL server, the connection test was successful with my visual basic form. however when I try putting in data to the database I get the error on line 14

System.Data.SqlClient.SqlException: 'Incorrect syntax near ')'.'

Here is my code: where have I gone wrong?

Imports System.Data.SqlClient
Public Class Form1
Dim name, hello, eligible As String
Dim blood As String
Dim agree As String

Dim connection As New SqlConnection("Server = DESKTOP-SNVR5AC; Database = bloodform; Integrated security = true")
Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click
    'connection
    Dim command As New SqlCommand("insert into form(Full_name,DOB,Email,Town,Blood_type,Age,Previous_donation,Any_diseases,Positive_bloodtest,Cardiac_problems,Bleeding_disorders) values ('" & TextBox1.Text & "','" & DateTimePicker1.Text & "','" & TextBox2.Text & "','" & ComboBox2.Text & "','" & ListBox2.Text & "','" & ComboBox1.Text & "','" & GroupBox1.Text & "','" & GroupBox2.Text & "','" & GroupBox3.Text & "','" & GroupBox4.Text & "','" & GroupBox5.Text & "',)", connection)

    connection.Open()

    If command.ExecuteNonQuery() = 1 Then
        MsgBox("Application submitted")
    Else
        MsgBox("Application not submitted")
    End If

    connection.Close()

    'checkbox
    If CheckBox1.Checked = False Then
        agree = "Please Agree the Terms and Conditions"
        MsgBox(agree)
    ElseIf CheckBox1.Checked = True Then

        'welcome message
        name = (TextBox1.Text)
        hello = "Welcome"
        MsgBox(hello & Space(2) & name)

        'age eligibility 
        If ComboBox1.SelectedIndex = 0 Then
            eligible = "You are underage"
            MsgBox(eligible)
        ElseIf ComboBox1.SelectedIndex = 1 Then
            eligible = "You can donate blood."
            'MsgBox(eligible)

            'blood
            If ListBox2.SelectedItem.ToString() = "Apositive" Then
                blood = "You are A+ and can donate to A+ and AB+"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "Bpositive" Then
                blood = "You are B+ and can donate to B+ and AB+"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "Opositive" Then
                blood = "You are O+ and can donate to O+, A+,  B+ and AB+"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "ABpositive" Then
                blood = "You are AB+ and can donate to AB+"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "Anegative" Then
                blood = "You are A- and can donate to  A+, A-, AB+ and AB-"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "Bnegative" Then
                blood = "You are B- and can donate to B+, B-, AB+ and AB-"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "Onegative" Then
                blood = "You are O- and can donate to Everyone"
                MsgBox(eligible & Space(1) & blood)
            ElseIf ListBox2.SelectedItem.ToString() = "ABnegative" Then
                blood = "You are AB- and can donate to AB+ and AB-"
                MsgBox(eligible & Space(1) & blood)

            End If
        End If
    End If
End Sub
End Class
jmcilhinney
  • 50,448
  • 5
  • 26
  • 46
KaurCodes
  • 7
  • 3
  • On the long query line, at the end, where you have `& GroupBox5.Text & "',)"` try removing the last comma just before the ")" `& GroupBox5.Text & "')"` – jamheadart Mar 18 '20 at 21:32
  • Yikes! This looks scary-vulnerable to sql injection issues. – Joel Coehoorn Mar 18 '20 at 21:43
  • 1
    This question demonstrates the age-old problem of having a syntax error in SQL code but not bothering to actually look at that SQL code. If you had bothered to look at `command.CommandText` then you'd have seen the actual SQL code, rather than just the VB code that builds it, and likely have seen your error. If not, at least you could have shown us the SQL code and we would have seen it more easily. The way you're doing it, it is hard to read and therefore error-prone, which is one of several reasons not to do it that way. – jmcilhinney Mar 18 '20 at 23:34

3 Answers3

1

The end of the SQL query contains an unnecessary comma

GroupBox4.Text & "','" & GroupBox5.Text & "',)", connection)

I recommend that you use sql parameters to pass the values, it is much safer (prevents sql injection), intelligent and easy to control.

https://stackoverflow.com/a/11139935/3195211

try:

cmd.CommandText = "INSERT INTO table2(column3) Values (@parameter ) where Column2=true"
cmd.Parameters.AddWithValue("@parameter", TextBox1.Text)
'etc.
ArthNRick
  • 925
  • 1
  • 7
  • 22
1

Some things to keep in mind:

It is not good to try to re-use the same connection object throughout a form. This interferes with a process called connection pooling.

It is not good to use string concatenation to put user input in an SQL query. In fact, it's difficult to overstate just how very bad that practice is.

The original code would have failed to close the connection if an exception was thrown.

You probably should check eligibility before inserting the record.

Dim connectionString As String = "Server = DESKTOP-SNVR5AC; Database = bloodform; Integrated security = true"

Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

    If Not CheckBox1.Checked Then
        MsgBox("Please agree to the Terms and Conditions")
        Exit Sub
    End If

    If ComboBox1.SelectedIndex = 0 Then
        MsgBox("You are underage")
        Exit Sub
    End If

    Dim SQL As String =
"INERT INTO FORM (
    Full_name, DOB, Email, Town, Blood_type, Age, Previous_donation, Any_diseases, Positive_bloodtest, Cardiac_problems, Bleeding_disorders
) VALUES (
  @FullNamae, @DOB, @Email, @Town, @BloodType, @Age, @PreviousDonation, @Diseases, @BloodTest, @Cardiac, @BleedingDisorders
)"

    Dim RowsChanged As Integer
    Using cn As New SqlConnection(connectionString), _
          cmd As New SqlCommand(SQL, cn)

        'I have to guess at types and lengths here. You should use actual types definitions from your database.
        cmd.Parameters.Add("@FullName", SqlDbType.NVarChar, 50).Value = TextBox1.Text
        cmd.Parameters.Add("@DOB", SqlDbType.DateTime).Value = DateTimePicker1.Value
        cmd.Parameters.Add("@Email", SqlDbType.NVarChar, 85).Value = TextBox2.Text
        cmd.Paramerers.Add("@Town", SqlDbType.NVarChar, 50).Value = ComboBox2.Text
        cmd.Parameters.Add("@BloodType", SqlDbType.VarChar, 3).Value = ListBox2.Text
        cmd.Parameters.Add("@Age", SqlDbType.Int).Value = Integer.Parse(ComboBox1.Text)
        cmd.Parameters.Add("@PreviousDonation", SqlDbType.VarChar, 1).Value = GroupBox1.Text
        cmd.Parameters.Add("@Diseases", SqlDbType.VarChar, 1).Value = GroupBox2.Text
        cmd.Parameters.Add("@BloodTest", SqlDbType.VarChar, 1).Value = GroupBox3.Text
        cmd.Parameters.Add("@Cardiac", SqlDbType.VarChar, 1).Value = GroupBox4.Text
        cmd.Parameters.Add("@BleedingDisorders", SqlDbType.VarChar, 1).Value = GroupBox5.Text

        cn.Open()
        RowsChanged = cmd.ExecuteNonQuery()
    End Using

    If RowsChanged <> 1 Then
        MsgBox("Application not submitted")
        Exit sub
    End If

    MsgBox($"Weclome  {TextBox1.Text}")

    Dim donorTable = New Dictionary(Of String, String) From
        {
            {"A+", "A+ and AB+"}, 
            {"B+", "B+ and AB+"}, 
            {"O+", "O+, A+,  B+ and AB+"}, 
            {"AB+", "AB+"}, 
            {"A-", "A+, A-, AB+ and AB-"}, 
            {"B-", "B+, B-, AB+ and AB-"}, 
            {"O-", "Everyone"}, 
            {"AB-", "AB+ and AB-"}, 
        }
    Dim bloodType As String = CStr(ListBox2.SelectedItem).
           Replace("positive", "+").Replace("negative", "-")
    Dim bloodDonor As String = donorTable(bloodType)

    MsgBox($"You are {bloodType} and can donate to {bloodDonor}")
End Sub
Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
0

To prevent SQL injection, please use parameterized queries instead.

Note that using the Using class can save you from using the Final method, and Close is done implicitly in ADO.NET via Dispose called by End Using.

Dim connectionString As String = "Server = DESKTOP-SNVR5AC; Database = bloodform; Integrated security = true"
Private Sub Button1_Click(sender As Object, e As EventArgs) Handles Button1.Click

    Using connection As New SqlConnection(connectionString)
        Try
            connection.Open()

            Dim command = New SqlCommand
            command.Connection = connection
            command.CommandText = "insert into form(Full_name) values (@parameter)"
            command.Parameters.AddWithValue("@parameter", TextBox1.Text)

            If command.ExecuteNonQuery() Then
                MsgBox("Application submitted")
            Else
                MsgBox("Application not submitted")
            End If
            command.Dispose()

        Catch ex As Exception
            MessageBox.Show("Error while inserting record on table..." & ex.Message, "Insert Records")
        End Try
    End Using
    ...

End Sub
Julie Xu-MSFT
  • 334
  • 1
  • 5
  • There are also some problems with AddWithValue(). You need to be careful with it. – Joel Coehoorn Mar 19 '20 at 13:34
  • Hi, i did try this, but I'm still getting an error when i fill my form and submit. i have checkboxes inside group boxes and their data type on the database is bit. so when i want to submit i get the error: Error while inserting record on table. conversion failed when converting the nvarchar value 'Have you ever donated blood before?' to data type bit. What should i do about that one? – KaurCodes Mar 19 '20 at 13:51
  • Open the connection directly before the Execute. Don't show message boxes with an open connection. You can include the command in the Using block. – Mary Mar 19 '20 at 22:02
  • @Joel Coehoorn,Yes, now I know difference between Parameters.Add(string, object) and Parameters.AddWithValue by this thread:https://stackoverflow.com/questions/9999751/difference-between-parameters-addstring-object-and-parameters-addwithvalue. – Julie Xu-MSFT Mar 20 '20 at 02:41