I have this piece of code for running a Stored Procedure, and I was wondering if there is a way of cutting code like:
uPar = .CreateParameter("@PropertyID", ADODB.DataTypeEnum.adInteger, ADODB.ParameterDirectionEnum.adParamInput) .Parameters.Append(uPar) .Parameters("@PropertyID").Value = Val(lblPropertyIDValue.Text)
As I have a lot of stored procedures in my system this takes a lot of time, my boss seems to think there could be a way to possible cut this down to one line of code and make it a lot easier to read and write, though I'm not sure what way to go about this.
Public Function SaveProperty() As Boolean
'** Save Current Personal Data Record
' Error Checking
On Error GoTo Err_SaveProperty
' Dimension Local Variables
Dim uRecSnap As ADODB.Recordset
Dim uPar As ADODB.Parameter
' Check For Open Connection
If uDBase Is Nothing Then
OpenConnection()
bConnection = True
End If
' Run Stored Procedure - Save Property Record
uCommand = New ADODB.Command
With uCommand
.ActiveConnection = uDBase
.CommandType = ADODB.CommandTypeEnum.adCmdStoredProc
.CommandTimeout = 0
uPar = .CreateParameter("@PropertyID", ADODB.DataTypeEnum.adInteger, ADODB.ParameterDirectionEnum.adParamInput)
.Parameters.Append(uPar)
.Parameters("@PropertyID").Value = Val(lblPropertyIDValue.Text)
uPar = .CreateParameter("@PropertyManager", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 60)
.Parameters.Append(uPar)
.Parameters("@PropertyManager").Value = cmbPropertyManager.Text
uPar = .CreateParameter("@AddressLine1", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@AddressLine1").Value = txtAddress1.Text
uPar = .CreateParameter("@AddressLine2", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@AddressLine2").Value = txtAddress2.Text
uPar = .CreateParameter("@AddressLine3", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 20)
.Parameters.Append(uPar)
.Parameters("@AddressLine3").Value = txtAddress2.Text
uPar = .CreateParameter("@Town", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@Town").Value = txtTown.Text
uPar = .CreateParameter("@PostCode", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@PostCode").Value = txtPostCode.Text
uPar = .CreateParameter("@Availabilty", ADODB.DataTypeEnum.adDate, ADODB.ParameterDirectionEnum.adParamInput)
.Parameters.Append(uPar)
.Parameters("@Availabilty").Value = chkAvailable.Checked
uPar = .CreateParameter("@Available", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@Available").Value = dtpAvailable.Text
uPar = .CreateParameter("@Factored", ADODB.DataTypeEnum.adTinyInt, ADODB.ParameterDirectionEnum.adParamInput)
.Parameters.Append(uPar)
.Parameters("@Factored").Value = -chkFactored.Checked
uPar = .CreateParameter("@FactorsName", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 20)
.Parameters.Append(uPar)
.Parameters("@FactorsName").Value = txtFactorName.Text
uPar = .CreateParameter("@FactorsEmail", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@FactorsEmail").Value = txtFactorsEmail.Text
uPar = .CreateParameter("@PropertyBuilt", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@PropertyBuilt").Value = dtpPropertyBuilt.Text
uPar = .CreateParameter("@PropertyValue", ADODB.DataTypeEnum.adDate, ADODB.ParameterDirectionEnum.adParamInput)
.Parameters.Append(uPar)
.Parameters("@PropertyValue").Value = txtPropertyValue.Text
uPar = .CreateParameter("@MimimumFee", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@MimimumFee").Value = txtMinimumFee.Text
uPar = .CreateParameter("@Commission", ADODB.DataTypeEnum.adTinyInt, ADODB.ParameterDirectionEnum.adParamInput)
.Parameters.Append(uPar)
.Parameters("@Commission").Value = -txtCommision.Text
uPar = .CreateParameter("@CostSuthorisationAmount", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 20)
.Parameters.Append(uPar)
.Parameters("@CostSuthorisationAmount").Value = txtCostAuthorisationAmount.Text
uPar = .CreateParameter("@Vacant", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@Vacant").Value = chkVacant.Checked
uPar = .CreateParameter("@VacantDate", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30)
.Parameters.Append(uPar)
.Parameters("@VacantDate").Value = dtpVacant.Text
uPar = .CreateParameter("@StartingRent", ADODB.DataTypeEnum.adDate, ADODB.ParameterDirectionEnum.adParamInput)
.Parameters.Append(uPar)
.Parameters("@StartingRent").Value = txtStartingRent.Text
.CommandText = "PropertyMaster_SaveRecord"
.Execute()
End With
' Close Connection
uRecSnap = Nothing
uCommand = Nothing
If bConnection Then CloseConnection()
SaveProperty = True
Err_SaveProperty:
If Err.Number <> 0 Then
sErrDescription = Err.Description
WriteAuditLogRecord("clsProperty", "SaveProperty", "Error", sErrDescription)
SaveProperty = False
End If
End Function
-
\$\begingroup\$ vb6 sorry didnt mean net \$\endgroup\$Dave123432– Dave1234322015年01月23日 10:14:29 +00:00Commented Jan 23, 2015 at 10:14
-
\$\begingroup\$ I think this solution outlined would simplify your needs. Thanks to one of the response from stackoverflow: stackoverflow.com/questions/2557606/… \$\endgroup\$Siva Karthikeyan– Siva Karthikeyan2015年01月23日 12:07:12 +00:00Commented Jan 23, 2015 at 12:07
-
1\$\begingroup\$ You need to see this: codereview.stackexchange.com/questions/46312/… \$\endgroup\$Mathieu Guindon– Mathieu Guindon2015年01月23日 13:25:57 +00:00Commented Jan 23, 2015 at 13:25
-
\$\begingroup\$ I've uploaded these handy classes to GitHub: github.com/retailcoder/VBTools \$\endgroup\$Mathieu Guindon– Mathieu Guindon2015年01月23日 14:41:04 +00:00Commented Jan 23, 2015 at 14:41
1 Answer 1
First, your question, then something else I noticed. You have a few options to clean this up. There's the easy way and the robust way. Let's look at the easy way first so you can make your boss happy.
The Question
Option 1: Instant Gratification
You can set the value of the parameter at the same you create it. This will remove one line of code for every parameter you have. For example:
uPar = .CreateParameter("@PropertyManager", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 60) .Parameters.Append(uPar) .Parameters("@PropertyManager").Value = cmbPropertyManager.TextBecomes:
uPar = .CreateParameter("@PropertyManager", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 60, cmbPropertyManager.Text) .Parameters.Append(uPar)You can one-line the creation and appending of each parameter. This will improve readability and do away with the
uParvariable entirely. It comes along with slightly more difficult debugging though. Like all things, it's a trade off..Parameters.Append .CreateParameter("@PropertyID", ADODB.DataTypeEnum.adInteger, ADODB.ParameterDirectionEnum.adParamInput, , Val(lblPropertyIDValue.Text)) .Parameters.Append .CreateParameter("@PropertyManager", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 60, cmbPropertyManager.Text) .Parameters.Append .CreateParameter("@AddressLine1", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30, txtAddress1.Text) .Parameters.Append .CreateParameter("@AddressLine2", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 30, txtAddress2.Text) ' etc....
Option 2: Create A Custom Class
Whenever we work with a class that has a terrible API, some times it makes a lot of sense to wrap that class in one of our own making. Then we get to define the API that we would like to work with ourselves. I won't go into detail about how I would go about this, but I will refer you to several other people who have experienced the same problem you're coming up against. Perhaps their work will inspire your own, or maybe you can down right (削除) steal (削除ここまで) use their code. (It's just a joke, the code I'm linking to is licensed CC-BY-SA. You're totally free to use it.)
- How do I associate Parameters to Command objects in ADO with VBScript?
- Creating ADODB Parameters on the fly
Review
This is Code Review after all, so I'd be remiss not to actually review your code a bit.
- Drop the Hungarian notation. The IDE will tell you what type a variable is. Perhaps it's a company standard, but unless you're working with vbscript, this kind of notation is ill advised. The biggest reason is because if the type of a variable changes, you either have to change the variable's name or let the notation lie to you. It's unnecessary maintenance and confusion.
- If you just have to keep using this notation, at least do it right. It seems that you use
uas a prefix for all objects. Don't do that. Be specific and explicit.cmdis proper for a command,prmfor a parameter etc. Doing this would force you away from the genericuParanduCommandnames and into something more descriptive. Win-win. I'm a little concerned about how you're keeping track of the state of the connection.
' Check For Open Connection If uDBase Is Nothing Then OpenConnection() bConnection = True End If ' lots of code If bConnection Then CloseConnection()This boolean doesn't actually tell you if the connection is open or not. Something could go wrong with the
OpenConnection()sub. It would be better to actually return a reference to the connection and check its state explicitly.
-
\$\begingroup\$ Why is the .Parameters.Append come with error \$\endgroup\$Dave123432– Dave1234322015年01月23日 15:14:15 +00:00Commented Jan 23, 2015 at 15:14
-
\$\begingroup\$ no accessible append accepts this number of arguments \$\endgroup\$Dave123432– Dave1234322015年01月23日 15:15:10 +00:00Commented Jan 23, 2015 at 15:15
-
\$\begingroup\$ You have to define a length for this line.
.Parameters.Append .CreateParameter("@PropertyID", ADODB.DataTypeEnum.adInteger, ADODB.ParameterDirectionEnum.adParamInput, , Val(lblPropertyIDValue.Text))should be something like.Parameters.Append .CreateParameter("@PropertyID", ADODB.DataTypeEnum.adInteger, ADODB.ParameterDirectionEnum.adParamInput, 50, Val(lblPropertyIDValue.Text))\$\endgroup\$RubberDuck– RubberDuck2015年01月23日 15:20:20 +00:00Commented Jan 23, 2015 at 15:20 -
\$\begingroup\$ I copied your code, so I'm surprised you're just now seeing this error. \$\endgroup\$RubberDuck– RubberDuck2015年01月23日 15:20:38 +00:00Commented Jan 23, 2015 at 15:20
-
\$\begingroup\$ yeah i Spotted that but it does not seem to fix the problem I am getting the error on the the .Parameters.Append saying no accessible append accepts this number of arguments, im fairly new to this so i appreciate the advice \$\endgroup\$Dave123432– Dave1234322015年01月23日 15:27:34 +00:00Commented Jan 23, 2015 at 15:27