views:

88

answers:

2

I ran into something odd, and I'm not precisely sure why it is behaving this way. In a for each loop I am adding rows to a table for a cross reference. Using the following code:

For Each cp In pCheckPoints
    If cp <> String.Empty Then
        Dim insertSQL As New StringBuilder
        With insertSQL
            .Append("INSERT INTO [CheckpointMessage] ( ")
            .Append(" [MessageID] ")
            .Append(", [CheckPoint] ")
            .Append(" ) VALUES ( ")
            .Append(" @MessageID ")
            .Append(", @Checkpoint ")
            .Append(" ) ")
        End With
        Using objCommand As New SqlCommand(insertSQL.ToString, MySQLConnection)
            With objCommand.Parameters
                .AddWithValue("@MessageID", pMessageID)
                .AddWithValue("@Checkpoint", cp)
            End With
            objCommand.ExecuteNonQuery()
            objCommand.CommandText = String.Empty
        End Using
    End If
Next

Without the objCommand.CommandText = String.Empty line the CommandText is appending the insertSQL but that doesn't make any sense to me because I would expect the objCommand's commandText to be empty since it is in a using block.

A: 

Other than the original question, the following lines need not be in the loop

Dim insertSQL As New StringBuilder

        With insertSQL
            .Append("INSERT INTO [CheckpointMessage] ( ")
            .Append(" [MessageID] ")
            .Append(", [CheckPoint] ")
            .Append(" ) VALUES ( ")
            .Append(" @MessageID ")
            .Append(", @Checkpoint ")
            .Append(" ) ")
        End With

        Using objCommand As New SqlCommand(insertSQL.ToString, MySQLConnection)

You could use the string once to create a command with parameterized query and use the same objCommand instance in the loop. The only thing that belongs in the loop is the dynamic values.

shahkalpesh
+3  A: 

Your command text is the same every time. Don't rebuild it. Try this:

Dim insertSql As String = _
    "INSERT INTO [CheckpointMessage] " & _
        "([MessageID], [CheckPoint]) " & _
        "VALUES " & _
        "(@MessageID, @ChceckPoint)"

Using cmd As New SqlCommand(insertSql, MySQLConnection)
    cmd.Parameters.Add("@MessageID", SqlDbType.Int).Value = pMessageID
    cmd.Parameters.Add("@CheckPoint", SqlDbType.NVarChar, 255) ''# I had to guess at this type

    For Each cp As String In pCheckPoints.Where(Function(c) Not String.IsNullOrEmpty(c))
        cmd.Parameters("@CheckPoint").Value = cp
        cmd.ExecuteNonQuery()
    Next cp
End Using

It's better for a lot of reasons:

  • The compiler can optimize your string concatenations away, where the StringBuilder forced it do that work at run time
  • Explicitly typed parameters avoid a few edge cases that can really kill performance in sql server, or even break your query.
  • This only creates your insert query string once, not once per checkpoint
  • This only creates one SqlCommand object
Joel Coehoorn
Ahh, of course. That makes way more sense anyway.
Anthony Potts