Passing multiple parameters to an SQL query

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; Da

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

StackExchange Code Review Q#144063, answer score: 6