CODEWITHSUNDEEP
sqlvb.netvisual-studio-2010
Mark A.
Mark A.

Reputation: 243

Unable to INSERT even if the record still does not exist in the database

I want to insert records in the database but before it inserts, it must first check the database whether the value being inserted already exists. Now my problem is that I am unable to insert into the database even though the value still does not exist.

Here's my code:

Dim check As New SqlCommand
    Dim sqlcheck As String = "SELECT SerialNumber FROM EquipmentDetail WHERE SerialNumber = '" & TextBox1.Text & "'"


    connection.Open()
    check.Connection = connection
    check.CommandText = sqlcheck

    Dim read As SqlDataReader = check.ExecuteReader()

    If read.HasRows Then

        While read.Read()

            If read("SerialNumber").ToString() = TextBox1.Text Then

                MessageBox.Show(read("SerialNumber").ToString() & " was already added")

            Else

                cmd.Connection = connection
                cmd.CommandText = "INSERT INTO EquipmentDetail (SerialNumber, BoxNumber, FATTNumber, FaultTicketNumber, Description, ProductCode)" &
                "VALUES('" & TextBox1.Text & "',  '" & TextBox2.Text & "',  '" & TextBox3.Text & "',  '" & TextBox4.Text & "', '" & TextBox6.Text & "',  '" & ComboBox1.Text & "')"
                cmd.ExecuteNonQuery()

                MessageBox.Show("Equipment successfully added.", "Equipment", MessageBoxButtons.OK)
                TextBox1.Text = ""
                TextBox2.Text = ""
                TextBox3.Text = ""
                TextBox4.Text = ""
                TextBox6.Text = ""
                TextBox1.Focus()
                connection.Close()

            End If
        End While
    End If
    read.Close()

Upvotes: 0

Views: 663

Answers (3)

competent_tech
competent_tech

Reputation: 44931

There are a number of issues with your code.

First, you have an sql injection vulnerability which can be resolved using parameters.

Second, you are performing database activities within an open datareader, which could lead to numerous issues.

Third, you are performing far too much work to determine whether or not the serial number exists. You can make use of the SQL Server Exists statement and the SqlCommand's ExecuteScalar method to return and test a single value, which makes the code much faster and easier to understand.

Finally, you need to ensure that disposable items are disposed and the easiest/best way to do this is through the use of Using blocks.

These can all be resolved with the following code:

    Using connection As New SqlConnection(connStr)
        Dim sqlcheck As String = "IF EXISTS(select 1 FROM EquipmentDetail WHERE SerialNumber=@SerialNumber) SELECT 1 ELSE SELECT 0"
        Using cmd As New SqlCommand(sqlcheck, connection)
            cmd.Parameters.AddWithValue("@SerialNumber", TextBox1.Text)
            connection.Open()

            If CBool(cmd.ExecuteScalar) Then
                MessageBox.Show(TextBox1.Text & " was already added")
            Else
                Using insertCmd As New SqlCommand
                    insertCmd.Connection = connection

                    insertCmd.CommandText = "INSERT INTO EquipmentDetail (SerialNumber, BoxNumber, FATTNumber, FaultTicketNumber, Description, ProductCode)" &
                    "VALUES(@SerialNumber, @BoxNumber, @FATTNumber, @FaultTicketNumber, @Description, @ProductCode)"
                    insertCmd.Parameters.AddWithValue("@SerialNumber", TextBox1.Text)
                    insertCmd.Parameters.AddWithValue("@BoxNumber", TextBox2.Text)
                    insertCmd.Parameters.AddWithValue("@FATTNumber", TextBox3.Text)
                    insertCmd.Parameters.AddWithValue("@FaultTicketNumber", TextBox4.Text)
                    insertCmd.Parameters.AddWithValue("@Description", TextBox6.Text)
                    insertCmd.Parameters.AddWithValue("@ProductCode", ComboBox1.Text)

                    insertCmd.ExecuteNonQuery()

                    MessageBox.Show("Equipment successfully added.", "Equipment", MessageBoxButtons.OK)
                    TextBox1.Text = ""
                    TextBox2.Text = ""
                    TextBox3.Text = ""
                    TextBox4.Text = ""
                    TextBox6.Text = ""
                    TextBox1.Focus()
                End Using

            End If
        End Using
        connection.Close()
    End Using

Upvotes: 2

PinnyM
PinnyM

Reputation: 35533

Firstly, you are concatenating strings to create a SQL statement. VERY dangerous, read up on SQL injection and parameterization.

The problem appears to be because you are only performing the INSERT only if the result from the SerialNumber check has rows. But if the SerialNumber isn't present, it won't do anything. You should probably try to modify the If statement like this:

If read.HasRows Then

    MessageBox.Show(read("SerialNumber").ToString() & " was already added")

Else

    cmd.Connection = connection
    cmd.CommandText = "INSERT INTO EquipmentDetail (SerialNumber, BoxNumber, FATTNumber, FaultTicketNumber, Description, ProductCode)" &
      "VALUES('" & TextBox1.Text & "',  '" & TextBox2.Text & "',  '" & TextBox3.Text & "',  '" & TextBox4.Text & "', '" & TextBox6.Text & "',  '" & ComboBox1.Text & "')"
    cmd.ExecuteNonQuery()

    MessageBox.Show("Equipment successfully added.", "Equipment", MessageBoxButtons.OK)
            TextBox1.Text = ""
            TextBox2.Text = ""
            TextBox3.Text = ""
            TextBox4.Text = ""
            TextBox6.Text = ""
            TextBox1.Focus()
            connection.Close()

End If
read.Close()

Upvotes: 0

El Kabong
El Kabong

Reputation: 717

If the SerialNumber value doesn't exist in the db, there won't be anything returned - so the block that contains your insert will never be reached.

Upvotes: 0

Related Questions