Accessing Collection
items by index is definitely a performance issue. Collections want to be iterated in a For Each
loop! If you know in advance how many items you'll need, best use an array; accessing array items by index is exactly what arrays do best (and that's why they're best iterated with a For
loop).
Writing to a Range
in a loop is also highly inefficient.
Now, you're not dumping collection/array items into a Range
- you're looking up key/value pairs. The single most efficient way to do this is with a Dictionary
. A Collection
can be keyed (as you did) too, but I like calling a cat a cat, so I use a Dictionary
for key-value pairs.
Note: I'm going to assume your key/value pairs are account/currency. Adjust as needed; the idea is to name things, so that the code speaks for itself.
You could have a Private Function CreateAccountsByCurrencyDictionary
that creates, populates and returns a Dictionary
, and then your macro could have a Static
local variable (so that it's not uselessly re-initialized every time the macro is invoked) to hold it:
Static accountsByCurrency As Scripting.Dictionary 'reference Microsoft Scripting Runtime
If accountsByCurrency Is Nothing Then
Set accountsByCurrency = CreateAccountsByCurrencyDictionary
End If
Then you grab your working range and dump it into a 2D array - the simplest way is to have your data live in a ListObject
(i.e. a named table); you can easily convert your range into a table by selecting "format as table" from the Home Ribbon tab - then you don't need to track where the last row is, the table does it for you!
Here Sheet1
is the code name of the worksheet you need to work with. Always qualify Range
calls with a specific worksheet object. By using the sheets' code name, you make your code work regardless of what the ActiveSheet
is.
Dim target As Range
Set target = Sheet1.ListObjects("TableName").DataBodyRange
Dim values As Variant
values = target.Value
Now that you have a 2D array (values
), iterate it with a For
loop and do your lookups:
Dim currentRow As Long
For currentRow = LBound(values, 1) To UBound(values, 1)
' never assume you're looking at valid data
Dim currentKeyValue As Variant
currentKeyValue = values(currentRow, 1)
Debug.Assert Not IsError(currentKeyValue) ' there's a problem in the data
' key is a valid string, but might not exist in the lookup dictionary
Dim currentKey As String
currentKey = currentKeyValue
If accountsByCurrency.Exists(currentKey) Then
' lookup succeeded, update the array:
values(currentRow, 1) = accountsByCurrency(currentKey)
Else
Debug.Print "Key not found: " & currentKey, "Index: " & currentRow
Debug.Assert False ' dictionary is missing a key. what now?
End If
Next
If all goes well the values
array now contains your corrected values, you can update the actual worksheet - and since you have the values in a 2D array, that's a single instruction!
target.Value = values
The CreateAccountsByCurrencyDictionary
function might look something like this:
Private Function CreateAccountsByCurrencyDictionary() As Scripting.Dictionary
Dim result As Scripting.Dictionary
Set result = New Scripting.Dictionary
With result
.Add "AUD", "120000037650264"
.Add "CAD", "140000028802654"
'...
End With
Set CreateAccountsByCurrencyDictionary = result
End Function
Or, the values could be populated from another worksheet table instead of being hard-coded. Point being, how the lookup values are acquired is a concern in its own right, and belongs in its own scope/procedure/function.