Building an odbc command string in Visual Basic-CodePudding

I'm trying to figure out where I am going wrong with the following SQL string in VB.NET

Dim SQL As String = "INSERT INTO USERS (" & String.Join(",", PropertyNames) & ", auditmonth) VALUES ('" & String.Join("','", Values) & "',", MonthName & "'"))

I'm getting the following three errors:

End of statement expected

Variable 'MonthName' hides a variable in an enclosing block

Unused local variable 'MonthName'

If I change back to

Dim SQL As String = "INSERT INTO USERS (" & String.Join(",", PropertyNames) VALUES ('" & String.Join("','", Values) & "')"

Then everything works fine. But what I'm trying to do is just add the current month to the database entry. The variable MonthName gets populated successfully. I'm just screwing something up with the syntax of the SQL command.

CodePudding user response：

You should try breaking it up to multiple lines, and debug it to review the whole string constructed:

"INSERT INTO USERS (" &
                String.Join(",", PropertyNames) &
                ", auditmonth) VALUES ('" &
                String.Join("','", Values) & "','" &
                MonthName & "')"

If your VB.NET compiler supports it, use this string substitution syntax for better readability:

String.Format(
                "INSERT INTO USERS ({0}, auditmonth) VALUES ('{1}','{2}')",
                String.Join(",", PropertyNames),
                String.Join("','", Values),
                MonthName)

and, do as @vku says!

CodePudding user response：

You really should use SQL parameters to pass the values. If you try to concatenate them in a string, it will break if there is an apostrophe in the value, and it's also vulnerable to SQL injection attacks.

As shown in tinamzu's answer, it is better to spread the code out over several lines to make it easier to read. Also, use as many variables as you like to keep each line simple.

So, you might have something like this:

Dim columnNames = String.Join(",", propertyNames)
Dim valuePlaceholders = String.Join(", ", Enumerable.Repeat("?", values.Count))

Dim sql = String.Format("INSERT INTO USERS ({0}, auditmonth) VALUES ({1}, ?)",
                        columnNames,
                        valuePlaceholders)

Using conn As New OleDbConnection("yourConnectionString"),
       cmd As New OleDbCommand(sql, conn)

    For Each v In values
        cmd.Parameters.Add("?", OleDbType.VarChar).Value = v
    Next

    cmd.Parameters.Add("?", OleDbType.VarChar).Value = monthName

    conn.Open()
    cmd.ExecuteNonQuery()

End Using

(Change the OleDbType.VarChar to match the relevant database column types.)

If you are using version 2015 or later of Visual Studio, you could use:

Dim sql = $"INSERT INTO USERS ({columnNames}, auditmonth) VALUES ({valuePlaceholders}, ?)"

instead as it is clear using just one line.