Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link
Source Link
RubberDuck
  • 31.1k
  • 6
  • 73
  • 176

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

  1. 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.Text

Becomes:

 uPar = .CreateParameter("@PropertyManager", ADODB.DataTypeEnum.adLongVarChar, ADODB.ParameterDirectionEnum.adParamInput, 60, cmbPropertyManager.Text)
 .Parameters.Append(uPar)
  1. You can one-line the creation and appending of each parameter. This will improve readability and do away with the uPar variable 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.)

#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 , 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 u as a prefix for all objects. Don't do that. Be specific and explicit. cmd is proper for a command, prm for a parameter etc. Doing this would force you away from the generic uPar and uCommand names 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.

lang-vb

AltStyle によって変換されたページ (->オリジナル) /