I have managed to write some pretty terrible code in VBA and I know there has to be a better way. I have made all of these variables to store some data which I eventually reference in the application. I’m fairly certain I need to loop over the recordset or fields and store the variables that way but I can’t figure out how to do it in a way that actually simplifies this code. It’s torturing me because it seems like a painfully simple concept.

The columns of the table that it is gathering data from looks similar to this:

[LANGUAGE_ID]-[NEXT]-[PREVIOUS]-[RESUME] — and so on

sqlstmt = "SELECT *" & _
" FROM cf_Language" & _
" WHERE LANGUAGE_ID = " & parm_language & ";"

Set rst_t = dbs.OpenRecordset(sqlstmt, dbOpenSnapshot)

If (parm_language <> 1) Then
    parm_surveyname = UnicodeToAscii(parm_surveyname, True)
    aNext = UnicodeToAscii(rst_t![Next], True)
    aPrevious = UnicodeToAscii(rst_t![previous], True)
    aResume = UnicodeToAscii(rst_t![resume], True)
    aFinish = UnicodeToAscii(rst_t![finish], True)
    aSave = UnicodeToAscii(rst_t![Save], True)
    aLogIn = UnicodeToAscii(rst_t![log_in], True)
    aComplete = UnicodeToAscii(rst_t![complete], True)
    aHeader = UnicodeToAscii(rst_t![header], True)
    aThanks = UnicodeToAscii(rst_t![thanks], True)
    aSavedMsg = UnicodeToAscii(rst_t![saved_msg], True)
    aBlankCode = UnicodeToAscii(rst_t![blank_code], True)
    aInvalidCode = UnicodeToAscii(rst_t![invalid_code], True)
    aTimeOut = UnicodeToAscii(rst_t![timeout], True)
    aClosed = UnicodeToAscii(rst_t![closed], True)
    aAccessCode = UnicodeToAscii(rst_t![access_code], True)
    aConFirstTime = UnicodeToAscii(rst_t![con_first_time], True)
    aAnonFirstTime = UnicodeToAscii(rst_t![anon_first_time], True)
    aAnonResume = UnicodeToAscii(rst_t![anon_resume], True)
Else
    aNext = rst_t![Next]
    aPrevious = rst_t![previous]
    aResume = rst_t![resume]
    aFinish = rst_t![finish]
    aSave = rst_t![Save]
    aLogIn = rst_t![log_in]
    aComplete = rst_t![complete]
    aHeader = rst_t![header]
    aThanks = rst_t![thanks]
    aSavedMsg = rst_t![saved_msg]
    aBlankCode = rst_t![blank_code]
    aInvalidCode = rst_t![invalid_code]
    aTimeOut = rst_t![timeout]
    aClosed = rst_t![closed]
    aAccessCode = rst_t![access_code]
    aConFirstTime = rst_t![con_first_time]
    aAnonFirstTime = rst_t![anon_first_time]
    aAnonResume = rst_t![anon_resume]
End If

Key Points

• I would not use local variables to hold recordset values, because you’d need one for every field of every returned row… which is just wrong.
• As already pointed out, good variable names are readable variable names (which can be pronounced!).
• You’re not showing it, but make sure you close that connection once you’re done.

“Entities”

Here’s an [perhaps less appealing] alternative to Dictionary. Since I learned about C# and started using Entity Framework, I like wrapping each record in an object – given a class called MyEntity (which does nothing more than exposing getters and setters for all properties):

Private Type ThisEntity
    SomeProperty As String
    AnotherProperty As String
End Type

Private this As ThisEntity
Option Explicit

Public Property Get SomeProperty() As String
    SomeProperty = this.SomeProperty
End Property

Public Property Let SomeProperty(value As String)
    this.SomeProperty = value
End Property

Public Property Get AnotherProperty() As String
    AnotherProperty = this.AnotherProperty
End Property

Public Property Let AnotherProperty(value As String)
    this.AnotherProperty = value
End Property

You define this object only once, and then you instantiate it as many times as you have records in your recordset. With a dictionary you get easier loading (each “entity” is a dictionary where you identify each field by their names to retrieve the corresponding value), but with this object you completely avoid referring to your properties using strings – the only hard-coded strings involved are the field names.

Reading the recordset

All you need to do is exactly as you said, loop through the recordset:

Dim entity As MyEntity
Dim entities As New Collection
While Not rs.EOF And Not rs.BOF 'BOF check if result has 0 rows

    Set entity = New MyEntity
    With entity
        .SomeProperty = rs.Fields("SomeField").Value 'or rs!SomeField
        .AnotherProperty = rs.Fields("AnotherField").Value 'or rs!AnotherField
    End With

    entities.Add entity ', entity.Id /PK property if you want to use a key here
    rs.MoveNext 'don't forget this, or your loop will be endless!
Wend

rs.Close
Set rs = Nothing
'at this point you have your entire recordset's content in the "entities" collection

About rs!FieldName notation (little “Gotcha!”)

For what it’s worth, I’ve recently fixed a bug in our codebase that was caused by the rs!FieldName notation in a specific circumstance: rs!FieldName was being added to a collection, and the recordset was left open. When I found that out, I added rs.Close and all of a sudden the collection, although non-empty, could no longer be read. This is because adding rs!FieldName to the collection (or, if you take the dictionary road) was actually adding a reference to the Field object, and the original programmer was left with the impression it worked fine because he could retrieve any entry in the collection and see the field’s value – when he was actually retrieving a Field object and through the default property was fetching the value from the recordset: that stopped working when I added code to close the recordset after all records were read.

That bug was fixed by explicitly adding rs.Fields(“FieldName”).Value to the collection instead.

From the help : “The default collection of a Recordset object is the Fields collection, and the default property of a Field object is the Value property. Use these defaults to simplify your code.”

I would either :

• Loop over the fields and create a dictionary with name/value pairs
• Make sure that rst_t is of type Snapshot and just keep using rst_t

Other than that:

• Hungarian notation, just don’t do it
• Disemvoweling ( sqlstmt ? ), just don’t do it either
• Seems like you could simply always call UnicodeToAscii and cut half of the code

On dictionaries in VBa:
https://stackoverflow.com/questions/915317/does-vba-have-dictionary-structure
On Hungarian notation ( and why it is bad ):
http://en.wikipedia.org/wiki/Hungarian_notation

I could not find a good article on disemvoweling ( remove all vowels from the name and then some like ‘statement’ -> ‘sttmnt’ -> ‘stmt’ ), I was sure Coding Horror had one. Basically it just makes your code harder to read for other people, it is one more barrier to (mis)understand code.