How to execute rd.HasRow properly without affecting uploading photo to database?

Question

I'm trying to use rd.HasRow method to validate whether the data typed in is duplicated or not before saving it to the database.

If it is duplicated, it is suppose to pop-up the error message box instead of saving the data.

How am I suppose to execute this along with the code I'm using to upload a photo to the database? If I comment this part of code, the typed in data (not duplicated) can be saved to database but the photo will not uploaded along with it.

'i = cmd.ExecuteNonQuery()
'If i >= 1 Then
    'MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
'Else
    'MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
'End If

But if I don't, the data typed in by the user will not be saved and this error message will pop-up against i=cmd.ExecuteNonQuery():

System.InvalidOperationException: 'There is already an open DataReader associated with this Command which must be closed first.'

This is the overall code.

Private Sub button2_Click(sender As Object, e As EventArgs) Handles button2.Click
    Dim con As New SqlConnection
    Dim cmd As New SqlCommand

    Dim rollno As String
    Dim name As String
    Dim gender As String
    Dim address As String
    Dim phoneno As Integer
    Dim datereg As String
    Dim faculty As String
    Dim course As String
    Dim semester As String
    Dim i As Integer
    Dim j As Integer

    rollno = TextBox1.Text
    name = TextBox2.Text
    gender = ComboBox4.Text
    address = TextBox3.Text
    phoneno = TextBox4.Text
    datereg = dateTimePicker1.Value
    faculty = comboBox1.Text
    course = comboBox2.Text
    semester = comboBox3.Text

    con.ConnectionString = "Data Source=LAPTOP-85ALBAVS\SQLEXPRESS;Initial Catalog=Portal;Integrated Security=True"
    cmd.Connection = con
    con.Open()

    'To validate whether duplication of typed in data by user occurs or not, if yes, error msg pop-up. If no, proceed and save the data into database
    Dim rd As SqlDataReader
    cmd.CommandText = "SELECT * FROM Profile WHERE RollNo= '" & TextBox1.Text & "' and Name='" & TextBox2.Text & "'"
    rd = cmd.ExecuteReader()
    If rd.HasRows Then
        MessageBox.Show("User already registered! Please try again.", "Error", MessageBoxButtons.OK)
    Else
        cmd.CommandText = "INSERT INTO Profile VALUES ('" & rollno & "' , '" & name & "' , '" & gender & "' , '" & address & "' , '" & phoneno & "' , '" & datereg & "' , '" & faculty & "' , '" & course & "' , '" & semester & "')"
    End If
    'i = cmd.ExecuteNonQuery()
    'If i >= 1 Then
    'MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
    'Else
    'MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
    'End If
    con.Close()

    con.Open()
    'To save the uploaded photo to table Photo

    Dim command As New SqlCommand("Insert into Photo (Img, Pid) Values (@Img, @Pid)", con)
    command.Connection = con
    Dim ms As New MemoryStream
    pictureBox1.Image.Save(ms, pictureBox1.Image.RawFormat)
    command.Parameters.Add("@Img", SqlDbType.Image).Value = ms.ToArray()
    command.Parameters.Add("@Pid", SqlDbType.VarChar).Value = TextBox1.Text

    j = cmd.ExecuteNonQuery()
    If j >= 1 Then
        MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
    Else
        MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
    End If
End Sub

Answer 1

The code looks a little messy and, in my experience at least, it can be difficult to debug messy code. There are a few things we can do to rectify that and I'll attempt to do that now with you.

First, give meaningful names to your controls. You can do this through the design on your form by selecting the control and changing the Name property. This will massively help you when referring to them through code. In this instance it will also help you eliminate the need for variables.

Consider implementing Using:

Sometimes your code requires an unmanaged resource, such as a file handle, a COM wrapper, or a SQL connection. A Using block guarantees the disposal of one or more such resources when your code is finished with them. This makes them available for other code to use.

This will help you manage your declarations and resources whilst also creating a clearer picture of your code.

I would also consider breaking each command into it's own Using block in an attempt to make your code clearer.

When inserting data into a database consider using SQL parameters to avoid SQL injection.

Finally onto the code, let's look at each Using block in turn.

First, I would start by initiating the SqlConnection within a Using block and then we can use that connection for each command:

Using con As New SqlConnection("Data Source=LAPTOP-85ALBAVS\SQLEXPRESS;Initial Catalog=Portal;Integrated Security=True")

    con.Open()

    'Add the rest of the code here

End Using

Checking the record exists:

Here, considering declaring a Boolean variable which we use to determine if the record exist.

Dim recordExists As Boolean = False
Using cmd As New SqlCommand("SELECT * FROM Profile WHERE RollNo = @RollNo AND Name = @Name", con)
    cmd.Parameters.Add("@RollNo", SqlDbType.[Type]).Value = txtRollNo.Text
    cmd.Parameters.Add("@Name", SqlDbType.[Type]).Value = txtName.Text

    Using reader As SqlDataReader = cmd.ExecuteReader()
        recordExists = reader.HasRows
    End Using
End Using

Show prompt if the record exists or insert into the database if it doesn't:

If recordExists Then
    MessageBox.Show("User already registered! Please try again.", "Error", MessageBoxButtons.OK)
Else
    Using cmd As New SqlCommand("INSERT INTO Profile VALUES (@RollNo, @Name, @Gender, @Address, @PhoneNo, @DateReg, @Faculty, @Course, @Semester)", con)

        cmd.Parameters.Add("@RollNo", SqlDbType.[Type]).Value = txtRollNo.Text
        cmd.Parameters.Add("@Name", SqlDbType.[Type]).Value = txtName.Text
        cmd.Parameters.Add("@Gender", SqlDbType.[Type]).Value = cboGender.Text
        cmd.Parameters.Add("@Address", SqlDbType.[Type]).Value = txtAddress.Text
        cmd.Parameters.Add("@PhoneNo", SqlDbType.[Type]).Value = txtPhoneNo.Text
        cmd.Parameters.Add("@DateReg", SqlDbType.[Type]).Value = dtpDateReg.Value
        cmd.Parameters.Add("@Faculty", SqlDbType.[Type]).Value = cboFaculty.Text
        cmd.Parameters.Add("@Course", SqlDbType.[Type]).Value = cboCourse.Text
        cmd.Parameters.Add("@Semester", SqlDbType.[Type]).Value = cboSemster.Text

        If cmd.ExecuteNonQuery() > 0 Then
            MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
        Else
            MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
        End If
    End Using
End If

Inserting the image:

Using cmd As New SqlCommand("INSERT INTO Photo (Img, Pid) VALUES (@Img, @Pid)", con)

    Using ms As New MemoryStream()
        pbxImage.Image.Save(ms, pbxImage.Image.RawFormat)

        cmd.Parameters.Add("@Img", SqlDbType.Image).Value = ms.ToArray()
        cmd.Parameters.Add("@Pid", SqlDbType.VarChar).Value = txtName.Text
    End Using

    cmd.ExecuteNonQuery()
End Using

Note that I have used SqlDbType.[Type] where I am unsure of your data type within the database. You will want to replace this with the data type you have specified for each column.

All together your code would look something like this:

Using con As New SqlConnection("Data Source=LAPTOP-85ALBAVS\SQLEXPRESS;Initial Catalog=Portal;Integrated Security=True")

    con.Open()

    Dim recordExists As Boolean = False
    Using cmd As New SqlCommand("SELECT * FROM Profile WHERE RollNo = @RollNo AND Name = @Name", con)
        cmd.Parameters.Add("@RollNo", SqlDbType.VarChar).Value = txtRollNo.Text
        cmd.Parameters.Add("@Name", SqlDbType.VarChar).Value = txtName.Text

        Using reader As SqlDataReader = cmd.ExecuteReader()
            recordExists = reader.HasRows
        End Using
    End Using

    If recordExists Then
        MessageBox.Show("User already registered! Please try again.", "Error", MessageBoxButtons.OK)
    Else
        Using cmd As New SqlCommand("INSERT INTO Profile VALUES (@RollNo, @Name, @Gender, @Address, @PhoneNo, @DateReg, @Faculty, @Course, @Semester)", con)

            cmd.Parameters.Add("@RollNo", SqlDbType.[Type]).Value = txtRollNo.Text
            cmd.Parameters.Add("@Name", SqlDbType.VarChar).Value = txtName.Text
            cmd.Parameters.Add("@Gender", SqlDbType.VarChar).Value = cboGender.Text
            cmd.Parameters.Add("@Address", SqlDbType.VarChar).Value = txtAddress.Text
            cmd.Parameters.Add("@PhoneNo", SqlDbType.VarChar).Value = txtPhoneNo.Text
            cmd.Parameters.Add("@DateReg", SqlDbType.VarChar).Value = dtpDateReg.Value
            cmd.Parameters.Add("@Faculty", SqlDbType.VarChar).Value = cboFaculty.Text
            cmd.Parameters.Add("@Course", SqlDbType.VarChar).Value = cboCourse.Text
            cmd.Parameters.Add("@Semester", SqlDbType.VarChar).Value = cboSemster.Text

            con.Open()

            If cmd.ExecuteNonQuery() > 0 Then
                MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
            Else
                MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
            End If
        End Using
    End If

    Using cmd As New SqlCommand("INSERT INTO Photo (Img, Pid) VALUES (@Img, @Pid)", con)

        Using ms As New MemoryStream()
            pbxImage.Image.Save(ms, pbxImage.Image.RawFormat)

            cmd.Parameters.Add("@Img", SqlDbType.Image).Value = ms.ToArray()
            cmd.Parameters.Add("@Pid", SqlDbType.VarChar).Value = txtName.Text
        End Using

        con.Open()

        cmd.ExecuteNonQuery()
    End Using

End Using

^{This code is untested, I haven't the environment but it should give you something to work with.}

Answer 2

Comments and explanations in line.

Private Sub OPCode()
        Dim i As Integer
        Dim j As Integer
        Dim rollno = TextBox1.Text
        Dim name = TextBox2.Text
        Dim gender = ComboBox4.Text
        Dim address = TextBox3.Text
        Dim phoneno = CInt(TextBox4.Text) 'Unless your phone numbers are very different
        'than the phone numbers here, the likelyhood of a user entering just numbers is
        'nil. Change this to a string and a VarChar in the database
        Dim datereg = dateTimePicker1.Value
        Dim faculty = comboBox1.Text
        Dim course = ComboBox2.Text
        Dim semester = ComboBox3.Text
        'The Using block ensures that your connection is closed and disposed
        'Pass your connection string to the constructor of the connection
        Using con As New SqlConnection("Data Source=LAPTOP-85ALBAVS\SQLEXPRESS;Initial Catalog=Portal;Integrated Security=True")
            'Pass the Sql command text and connection to the Constructor of the command.
            'NEVER, NEVER, NEVER allow user input to be passed directly to a database. Always use parameters.
            Dim cmd As New SqlCommand("SELECT * FROM Profile WHERE RollNo= @RollNo and [Name]= @Name;", con)
            cmd.Parameters.Add("@RollNo", SqlDbType.VarChar).Value = rollno
            cmd.Parameters.Add("@Name", SqlDbType.VarChar).Value = name
            con.Open()
            Using rd As SqlDataReader = cmd.ExecuteReader()
                'To validate whether duplication of typed in data by user occurs or not, if yes, error msg pop-up. If no, proceed and save the data into database
                If rd.HasRows Then
                    MessageBox.Show("User already registered! Please try again.", "Error", MessageBoxButtons.OK)
                    'You don't want to go any further if the user is registered.
                    Exit Sub
                End If
            End Using
            'Just use another new command variable to avoid confusion
            'I think it is much better practice to list the fields.
            Dim cmd2 As New SqlCommand("INSERT INTO Profile VALUES (@RollNo ,@Name,@Gender, @Address, @PhoneNo , @DateReg , @Faculty , @Course , @Semester);", con)
            cmd2.Parameters.Add() 'etc.
            i = cmd2.ExecuteNonQuery()
            If i >= 1 Then
                MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
            Else
                MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
                Exit Sub
            End If
            'To save the uploaded photo to table Photo
            Dim command3 As New SqlCommand("Insert into Photo (Img, Pid) Values (@Img, @Pid)", con)
            command3.Connection = con
            Dim ms As New MemoryStream
            pictureBox1.Image.Save(ms, pictureBox1.Image.RawFormat)
            command3.Parameters.Add("@Img", SqlDbType.Image).Value = ms.ToArray()
            command3.Parameters.Add("@Pid", SqlDbType.VarChar).Value = TextBox1.Text

            j = command3.ExecuteNonQuery()
        End Using
        If j >= 1 Then
                MessageBox.Show("Profile successfully registered!", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
            Else
                MessageBox.Show("Error. Please try again later.", "Message", MessageBoxButtons.OK, MessageBoxIcon.Information)
            End If
    End Sub