-2

I'm creating a registration form in MSVStudio and and error always appear. I already set each variable to their own datatype but same error prompt to me. "Conversion from string "The INSERT INTO statement contai" to type 'Boolean' is not valid."

here is my function in my class

  Function registercust(ByVal a As String, ByVal b As String, ByVal c As String, ByVal d As String, ByVal f As String, ByVal g As String, ByVal h As DateTime, ByVal i As String, ByVal j As String)
    Dim conn As New OleDb.OleDbConnection
    Dim dr As OleDb.OleDbDataReader
    Dim rs As New OleDb.OleDbCommand
    Try
        conn.ConnectionString = cs
        conn.Open()
        query = "Insert into custinfo (`custid`,`lastname`,`firstname`,`mi`,`address`,`telephone`,`birthday`,`age`,`status`) values('" & a & "','" & b & "','" & c & "','" & d & "','" & f & "','" & g & "','" & h & "','" & i & "','" & j & "')"
        rs = New OleDb.OleDbCommand(query, conn)
        dr = rs.ExecuteReader
        If dr.Read Then
            Return True
        Else
            Return False
        End If

    Catch ex As Exception
        Return ex.Message
    End Try
    conn.Close()
End Function

And Here is my code yo my Button

 Private Sub Button2_Click(ByVal sender As System.Object, ByVal e As System.EventArgs) Handles Button2.Click
    If class1.chkfieldss(TextBox14.Text, TextBox1.Text, _
                          TextBox8.Text, TextBox9.Text, _
                          TextBox10.Text, TextBox11.Text, TextBox12.Text) = False Then
        If class1.registercust(TextBox14.Text, TextBox1.Text, TextBox8.Text, TextBox9.Text, TextBox10.Text, _
                               TextBox11.Text, DateTimePicker1.Value, _
                               TextBox12.Text, ComboBox3.SelectedItem) = False Then

            MessageBox.Show("REGISTER SUCCESSFULLY", "Welcome", MessageBoxButtons.OK, MessageBoxIcon.Exclamation)
        Else
            MessageBox.Show("Something Happen", "Error", MessageBoxButtons.OK, _
                            MessageBoxIcon.Error)
        End If

    Else
        MessageBox.Show("Complete all fields", "Error", MessageBoxButtons.OK, _
                           MessageBoxIcon.Error)
    End If
End Sub
vin dicy
  • 21
  • 1
  • 5
  • 1
    Your apostrophes look odd on the insert's columns specified. The apostrophes on the values look normal. Aside from that, you should definitely user parameterized SQL – UnhandledExcepSean Oct 08 '15 at 18:16
  • Actually, your code is riddled with issues of all sorts. Please see my answer below to see at least some of these solved – T.S. Oct 08 '15 at 19:09
  • Your function is apparently designed to return a boolean but in the case of an exception, you attempt to return a string. Put `Option Strict On` at the top of your code or turn it on in your project's settings. Incidentally, if there is an exception, your `conn.Close()` will never be called. You should always work with a connection inside a `Using` block. – Chris Dunaway Oct 09 '15 at 14:04

3 Answers3

3

You're yet another victim of keeping the strict compiler option off. This should always be on.


You have not defined a return type for the function registercust. Should I expect a boolean or a string?

Function registercust(...) '<- As surprise?

Your query fails, probably due to the horrifying way you're creating the query. Always use prepared statements. The function returns the error message (string) "The INSERT INTO statement contai...".

Return ex.Message

Now, back in the Button2_Click method you're trying to compare the returned value to a boolean value.

If class1.registercust(...) = False Then

This is where your code breaks apart. You cannot convert the value "The INSERT INTO statement contai..." (string) to either False or True (boolean).

Community
  • 1
  • 1
Bjørn-Roger Kringsjå
  • 9,849
  • 6
  • 36
  • 64
1

There is an error in your insert statement. Use debug method to identify the error. It is always better to use Parameterized queries.

This will help prevent SQL injection attacks as well as help to omit the errors of concatenation such as quotation mark in the user input (')

haraman
  • 2,744
  • 2
  • 27
  • 50
0

This is how your function should look - see in code comments

' you should really pass customer model with properties here instead of all these arguments
Public Class Customer
    Public Property CustId As String
    Public Property LastName As String
    Public Property FirstName As String
    Public Property Mi As String
    Public Property Address As String
    Public Property Telephone As String
    Public Property Birthday As String
    Public Property Age As String
    Public Property Status As String
End Class
. . . . . . . . . 
Function RegisterCust(ByVal c As Customer) As Boolean '<-- add returning type

    Dim retVal As Boolean

    ' "Using" helps to dispose of objects that implement IDisposable
    ' Try to avoid oledb provider altogether for RDBMSs
    Using conn As OleDb.OleDbConnection = New OleDb.OleDbConnection(cs) ' pass conn str on creation
        ' This makes for clean code
        Dim query As String = "Insert into custinfo " & 
            "(custid, lastname, firstname, mi, address, telephone, birthday, age, status) values " & 
            "('{0}', '{1}', '{2}', '{3}', '{4}', '{5}', '{6}', '{7}', '{8}')" 
        query = String.Format(query, c.CustId, c.LastName, c.FirstName, c.Mi, c.Address, c.Telephone, c.Birthday, c.Age, c.Status)

        ' Now, this query above "all good and dandy" but it is vulnerable to sql injection. 
        ' To prevent it, instead of  '{1}', you would type something like @1, or if you want, @lastname - a parameter name
        ' Then, add a parameter with your value to cmd.parameter collection.

        Using cmd As OleDb.OleDbCommand = New OleDb.OleDbCommand(query, conn)

            conn.Open()

           ' your query doesn't need reader. You inserting value without returning anything 
           retVal = cmd.EcecuteNonQuery() > 0 ' Assign return value

       End Using ' cmd
       conn.Close()
    End Using ' conn

    return retVal
End Function

I removed exception handling because you didn't have a true handling. With using you at least safe that objects will close before this method exits. You can wrap this whole thing into try-block if you wish

This is not what you have to exactly do but it is a good format

T.S.
  • 18,195
  • 11
  • 58
  • 78
  • 1
    This code is still wide open to sql injection attacks. You should use a prepared query. – Chris Dunaway Oct 09 '15 at 14:06
  • @ChrisDunaway I agree but I never claimed that this code is perfect and bullet proof. It was an attempt to show one of the ways for cleaner code of the sort. Just about anyone with similar questions has this vulnerability written into it. I just can't continuously talk about it. So I threw it out of this answer. But to please you and whoever else, I am adding a comment pointing to this issue. Thank you – T.S. Oct 09 '15 at 14:48
  • If you're going to go through the trouble of rewriting the code to clean it up, just write the parameterized sql code while you're at it. It's barely any more effort. – Chris Dunaway Oct 09 '15 at 15:03