Passing multiple parameters to an SQL query

Question

I am trying to use this VBA code to pass a SQL stored procedure multiple values from an excel sheet.

In order to have the procedure run multiple times and insert multiple sets of information into the database, I copied and pasted the parameter section of the code each time I wanted to insert new data and hard coded which excel cell to pull data from.

Is there a way to consolidate the code so that I can reference cells A1:A10 for parameter 1, cells B1:B10 for parameter 2, cells C1:C10 for parameter 3 and so on without copying the code over again?

Sub Module2()
Dim cnSQL As ADODB.Connection 
Dim sqlCommand As ADODB.Command
Dim prm As ADODB.Parameter
Set cnSQL = New ADODB.Connection 
cnSQL.Open "Provider=SQLOLEDB.1; uid=test; Pwd=test; Initial Catalog = test; Data source=test"
Set sqlCommand = New ADODB.Command
sqlCommand.ActiveConnection = cnSQL
sqlCommand.CommandType = adCmdStoredProc
sqlCommand.CommandText = "EstimateInsertUpdate"
Set prm = sqlCommand.CreateParameter("CD", adVarChar, adParamInput, 10)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("CD").Value = Worksheets("Sheet1").Range("B5")
Set prm = sqlCommand.CreateParameter("Date", adDate, adParamInput)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("Date").Value = Worksheets("Sheet1").Range("F5")
Set prm = sqlCommand.CreateParameter("Test", adDecimal, adParamInput)
prm.Precision = 4
prm.NumericScale = 4
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("Test").Value = Worksheets("Sheet1").Range("E5")
Set prm = sqlCommand.CreateParameter("EDtm", adDate, adParamInput)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("EDtm").Value = Worksheets("Sheet1").Range("D5")
Set prm = sqlCommand.CreateParameter("UserID", adVarChar, adParamInput, 10)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("UserID").Value = Worksheets("Sheet1").Range("G5")
sqlCommand.Execute
Set cnSQL = New ADODB.Connection 
cnSQL.Open "Provider=SQLOLEDB.1; uid=test; Pwd=test; Initial Catalog = PMW; Data source=test"
Set sqlCommand = New ADODB.Command
sqlCommand.ActiveConnection = cnSQL
sqlCommand.CommandType = adCmdStoredProc
sqlCommand.CommandText = "EstimateInsertUpdate"
Set prm = sqlCommand.CreateParameter("CD", adVarChar, adParamInput, 10)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("CD").Value = Worksheets("Sheet1").Range("B6")
Set prm = sqlCommand.CreateParameter("Date", adDate, adParamInput)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("Date").Value = Worksheets("Sheet1").Range("F6")
Set prm = sqlCommand.CreateParameter("Test", adDecimal, adParamInput)
prm.Precision = 4
prm.NumericScale = 4
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("Test").Value = Worksheets("Sheet1").Range("E6")
Set prm = sqlCommand.CreateParameter("EDtm", adDate, adParamInput)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("EDtm").Value = Worksheets("Sheet1").Range("D6")
Set prm = sqlCommand.CreateParameter("UserID", adVarChar, adParamInput, 10)
sqlCommand.Parameters.Append prm
sqlCommand.Parameters("UserID").Value = Worksheets("Sheet1").Range("G6")
sqlCommand.Execute
End Sub

Answer 1

Sub Module2 is a terrible name to use for a procedure. Not only it doesn't say anything about what it does, it looks like the [default] name of some standard code module.

Name things, it's important. Procedures do something, their names should start with a verb, and you should be able to read it and know roughly what it does just by the name.

Consider:

Public Sub InsertOrUpdateEstimates()

Also indent your code - it's harder to read code that begins in column 1 no matter what.

Instead of this:

Sub DoSomething()
DoSomeThings
DoSomeMoreThings
End Sub

Consider:

Sub DoSomething()
 DoSomeThings
 DoSomeMoreThings
End Sub

---

You're defining 5 parameters per command, but declare only 1 ADODB.Parameter object variable; this changes the meaning of the variable as the procedure executes, and this makes for bug-prone and confusing code. If a variable stands for "the UserID parameter", then it's reasonable to expect that variable to mean the same thing throughout the scope of the procedure. Same for the connection objects.

The problem is exactly that: the scope of the procedure is too wide - it's doing too many things.

You've essentially written a script - it goes top to bottom and executes an ordered sequence of operations and then completes.

You don't need to "consolidate" the code, you need to tear it apart and extract each responsibility into its own function, break it down into multiple, smaller, more specialized procedures.

You want to run a command for each row of a given range: you need a procedure that runs a command for a single row, and call it from another procedure.

The command takes 5 parameters; make the procedure receive 5 values as ...parameters:

Private Sub InsertOrUpdateEstimate(ByVal CD as String, ByVal theDate As Date, ByVal test As Single, ByVal EDtm As Date, ByVal UserID As String)

The body of that procedure will need to create an ADODB.Parameter object for each parameter value: instead of repeating the code for that 5 times, make another function that's responsible for creating an ADODB parameter - something like this:

Private Function CreateCommandParameter(ByVal name As String, ByVal value As Variant, Optional ByVal numPrecision As Integer = 4, Optional ByVal numScale As Integer = 4) As ADODB.Parameter
 Dim result As New ADODB.Parameter
 result.Direction = adParamInput
 result.name = name
 result.value = value
 Select Case VarType(value)
 Case VbVarType.vbBoolean
 result.Type = adBoolean
 Case VbVarType.vbDate
 result.Type = adDate
 Case VbVarType.vbCurrency
 result.Type = adCurrency
 result.Precision = numPrecision
 result.NumericScale = numScale
 Case VbVarType.vbDouble
 result.Type = adDouble
 result.Precision = numPrecision
 result.NumericScale = numScale
 Case VbVarType.vbSingle
 result.Type = adSingle
 result.Precision = numPrecision
 result.NumericScale = numScale
 Case VbVarType.vbByte, VbVarType.vbInteger, VbVarType.vbLong
 result.Type = adInteger
 Case VbVarType.vbString
 result.Type = adVarChar
 Case Else
 Err.Raise 5, Description:="Data type not supported"
 End Select
 Set CreateCommandParameter = result
End Function

Now, InsertOrUpdateEstimate can look like this:

Private Sub InsertOrUpdateEstimate(ByVal conn As ADODB.Connection, ByVal CD As String, ByVal theDate As Date, ByVal test As Single, ByVal EDtm As Date, ByVal UserID As String)
 With New ADODB.Command
 .ActiveConnection = conn
 .CommandType = adCmdStoredProc
 .CommandText = "EstimateInsertUpdate"
 .Parameters.Append CreateCommandParameter("CD", CD)
 .Parameters.Append CreateCommandParameter("Date", theDate)
 .Parameters.Append CreateCommandParameter("Test", test)
 .Parameters.Append CreateCommandParameter("EDtm", EDtm)
 .Parameters.Append CreateCommandParameter("UserID", UserID)
 .Execute
 End With
End Sub

Notice it's receiving the ADODB.Connection as a parameter, so it's responsible for executing a command, but not for setting up the connection.

So all that's left to do is to call that procedure inside the body of some loop that iterates all the rows we want to look at.

But there's more to consider: what if an error occurs because some of the data in row 7 is of the wrong type? What if the connection times out?

It would be a good idea to initiate a transaction before we start looping, and only commit the transaction once all rows have been correctly processed - and if any error happens, we can rollback the entire transaction, fix things and start over.

Think of a transaction as a bank transaction: you're moving dollars from one account to the other - if something wrong happens in the middle of the transaction, you don't want the money to be withdrawn from the source account but not deposited into the target account! A transaction either runs completely, or not at all: wrapping all these insert/update commands in a transaction is probably a good idea.

To do this correctly you need to handle runtime errors. Consider this procedure:

Public Sub InsertOrUpdateEstimates()
 Const connString As String = "..."
 Const fromRow As Long = 1
 Const toRow As Long = 10
 Dim CD As String
 Dim theDate As Date
 Dim test As Single
 Dim EDtm As Date
 Dim UserID As String
 On Error GoTo CleanFail
 Dim conn As ADODB.Connection
 Set conn = New ADODB.Connection
 conn.Open connString
 conn.BeginTrans
 Dim currentRow As Long
 With Sheet1
 For currentRow = fromRow To toRow
 CD = .Cells(currentRow, 2).value
 EDtm = .Cells(currentRow, 4).value
 test = .Cells(currentRow, 5).value
 theDate = .Cells(currentRow, 6).value
 UserID = .Cells(currentRow, 7).value
 InsertOrUpdateEstimate conn, CD, theDate, test, EDtm, UserID
 Next
 End With
 conn.CommitTrans
CleanExit:
 conn.Close
 Exit Sub
CleanFail:
 conn.RollbackTrans
 MsgBox Err.Description & vbNewLine & "Transaction was rolled back, no changes were made", vbExclamation
 Resume CleanExit
End Sub

Notice the Sheet1 object is being referred to directly instead of being fetched from the Worksheets collection everytime it's needed; the With block removes all redundancies here. Also we're reading the values using the Cells function instead of Range, so we can easily supply a row/column number to pick up a single value.

---

You can push the segregation of responsibilities to the next level by moving the whole ADODB-handling code to its own class - see materializing any ADODB query and creating ADODB parameters on the fly for more information; the For loop above could look something like this (and that replaces the helper CreateCommandParameter function and CreateCommandParameter procedure, too):

Const sql As String = "exec dbo.EstimateInsertUpdate ?,?,?,?,?;"
conn.BeginTrans
With Sheet1
 For currentRow = fromRow To toRow
 SqlCommand.ExecuteNonQuery conn, sql, _
 .Cells(currentRow,2).value, _
 .Cells(currentRow,4).value, _
 .Cells(currentRow,5).value, _
 .Cells(currentRow,6).value, _
 .Cells(currentRow,7).value
 Next
End With
conn.CommitTrans

The fewer things a procedure does, the clearer the code becomes; there shouldn't ever be a reason to copy+paste code - if you find yourself pressing Ctrl+C, ask yourself "is there not a better way?" before you press Ctrl+V.

• mag123: Not sure I follow your thought process. In your example with runtime error handling, what would go in the ... of Const connString As String = "..."
• Mathieu Guindon: That's your connection string.
• mag123: Got it. When trying to run that code i am getting a compile error saying 'wrong number of arguments or invalid property assignment.' Any idea what could be causing that?
• Mathieu Guindon: Where? Verify what you're calling and whether you're giving it the correct number of arguments.
• mag123: I don't understand how you've broken up the code. Do I need each section of code that you wrote or are some sections substitutes for other sections?