# A 'flexible' VBA approach to lookups using arrays, scripting dictionary and user input

In my previous post Optimise compare and match method using scripting.dictionary in VBA I wanted to address optimising the scripting.dictionary approach I was using and I feel I have achieved that (feel free to correct me though!). I am now implementing user input boxes which have also increased the complexity and now have duplicated code. I'm wondering if there might be a better way of doing this?

This is all one sub procedure and it is working well as far as I have been able to test it, I have just broken it up with comments.

Public Sub arrayMatchv3()

Dim workingArray As Variant
Dim workingCell As Range
Dim workingLastRow As Long
Set workingCell = Application.InputBox("Select the first cell of your comparison column.", Type:=8)
workingLastRow = workingCell.End(xlDown).Row
With workingCell
End With

Dim targetArray As Variant
Dim targetCell As Range
Set targetCell = Application.InputBox("Select the first cell of your target column.", Type:=8)
With targetCell
End With

Dim i As Long


With the below ReDim section, I am unsure of any method to write a single column from the worksheet to a single column within a 2d array, so have resorted to looping to populate the second column. Perhaps it isn't really needed to add them both into a single array?

    ReDim Preserve workingArray(1 To UBound(workingArray, 1), 1 To UBound(workingArray, 2) + 1)
For i = LBound(workingArray, 1) To UBound(workingArray, 1)
workingArray(i, UBound(workingArray, 2)) = targetArray(i, 1)
Next i


The below three code sections are duplicates of the three above but using different variables, not sure how to address this?

    Dim comparisonArray As Variant
Dim comparisonCell As Range
Dim comparisonLastRow As Long
Set comparisonCell = Application.InputBox("Select the first cell of the column to match values with.", Type:=8)
comparisonLastRow = comparisonCell.End(xlDown).Row
With comparisonCell
End With

Dim returnArray As Variant
Dim returnCell As Range
Set returnCell = Application.InputBox("Select the first cell from the column you wish to return values from.", Type:=8)
With returnCell
End With

ReDim Preserve comparisonArray(1 To UBound(comparisonArray, 1), 1 To UBound(comparisonArray, 2) + 1)
For i = LBound(comparisonArray, 1) To UBound(comparisonArray, 1)
comparisonArray(i, UBound(comparisonArray, 2)) = returnArray(i, 1)
Next i


I'm quite happy with the remaining code which includes a lot of feedback from my previous posts, but critique is still welcome, especially if it can address any potential performance issues.

    With New Scripting.Dictionary
.CompareMode = vbTextCompare

For i = LBound(comparisonArray, 1) To UBound(comparisonArray, 1)
If Not .Exists(comparisonArray(i, 1)) Then
End If
Next i

Dim arrayToPaste As Variant
ReDim arrayToPaste(1 To UBound(workingArray, 1), 1 To 1)
For i = LBound(workingArray, 1) To UBound(workingArray, 1)
If Len(workingArray(i, 2)) = 0 Then
If .Exists(workingArray(i, 1)) Then
arrayToPaste(i, 1) = .Item(workingArray(i, 1))
End If
Else
arrayToPaste(i, 1) = workingArray(i, 2)
End If
Next i
End With

With targetCell
End With

End Sub


### Signature

Public Sub arrayMatchv3()


I like that the procedure is explicitly Public. Unlike a lot of other languages - including VB.NET, in VBA a member is public unless specified otherwise; being explicit is a good thing. I don't see Option Explicit atop your procedure, but I have it in my IDE and it compiles with it, so kudos for declaring all your variables!

If you're not specifying Option Explicit, you have to know that the slightest typo in a variable's name will make VBA "declare" an implicit Variant variable on-the-spot to receive the value you're assigning to what you intended to be that local variable you just declared: VBA happily compiles with undeclared identifiers, which inevitably will cause bugs that will be hard to find. With Option Explicit specified, the compiler will choke on a typo, finding them for you in a split second.

Member names in VBA should be PascalCase, so that the API you're writing feels like the API you're using when you write VBA code: the entire Excel object model, the VBA standard library, the scripting runtime library - pretty much every single VBA API out there, they all expose modules, classes, procedures, functions, properties, constants, enums, events, etc. - all in PascalCase.

I like that you're using camelCase for your locals though; I find it makes the code feel a bit easier to read, and I do it myself.

The name isn't ideal: "array match v3" is a noun - it's an "array match"... nouns make excellent module/class names: Worksheet, Workbook, Application. Procedures do something, they're verbs. If the procedure matches arrays then MatchArray would be a better name.

I see you're having a version number in the name: this supposes there's an arrayMatchv2 somewhere, and then an arrayMatchv1 or arrayMatch procedure not too far. If not, then just remove the v3 suffix.

Otherwise, you're storing versioning metadata in the code base itself - which is clutter: if arrayMatch usages have been replaced by references to arrayMatchv2, you now have as many places to revisit in the code to make sure it's now calling arrayMatchv3. If v3 fixed a bug in v2, your code base still has the bug even though v3 fixed it.

If you need versioning, use source control. You can create a repository on GitHub, and then use a tool such as Rubberduck to commit, revert, push, pull, create branches and merge them, straight from a docked toolwindow in the IDE. And then you can use GitHub to view any version, or git to fetch any specific version.

I guess that covers the method's signature =)

### User input and responsibilities

it is working well as far as I have been able to test it

To the outside world, the procedure is a complete black box: it takes no input and returns no value. Beyond the name and without peeking at the source code, we have no idea what's happening in there when we read the calling code.

Application.InputBox is called 4 times: there are 4 user inputs happening. By telling the outside world "I need 4 Range parameters to do my thing", the procedure's signature would already be telling us a lot more.

Collecting user input is a concern in its own right. All kinds of wild things can happen when a user gets involved. They can ESC out, or X-out of your dialogs, make typos, enter illegal values - many things can go wrong when a user needs to get involved.

Set workingCell = Application.InputBox("Select the first cell of your comparison column.", Type:=8)
workingLastRow = workingCell.End(xlDown).Row


Accessing workingCell.End here means the program dies with a runtime error if you (/the user) X-out or otherwise cancel the InputBox.

workingCell.End(xlDown) will stop at the first blank cell it finds: if the row can contain blank cells, your workingLastRow will not be the actual last row in the selected column. A more reliable way would be to start at the last row in workingCell's column, and go xlUp from there instead:

With workingCell.Parent
workingLastRow = .Cells(.Rows.Count, workingCell.Column).End(xlUp).Row
End With


Take the collect user input concern out of the procedure: use parameters, and make it the caller's responsibility to figure out how to get that input - with an InputBox... or perhaps just any arbitrary set of Range objects. Don't trust the input: either raise a descriptive error, or exit early if the input isn't valid (e.g. if any parameter is Nothing).

Except... but we aren't actually working with Range objects - what we really want is a workingArray, a targetArray, a comparisonArray, a returnArray... and we return a Dictionary. 4 parameters and a return value: the Sub becomes a Function, and the inputs and outputs become much clearer already. The fact that you felt the need to split it up in the OP speaks a lot about this concern, too.

At this point things are starting to shape up:

• A function to collect user input; validates and returns a Range object
• A function that takes a Range and returns an array
• A function that takes input arrays and returns a Dictionary
• A procedure that takes a Dictionary and writes to a specified Range
• A procedure that coordinates all those

By simply splitting things up and tightening variable lifetimes and scopes, and passing and returning objects and arrays, we've made it much easier to write code that can test various valid and invalid inputs and document how things behave... and automated tests are much more reliable than ad-hoc F5-debug testing.

### Bugs & Assumptions

Your code is making a bad assumption here, can you spot it?

With workingCell
End With


Range is implicitly referring to the active worksheet: the unqualified call will throw an error if the active sheet is a chart! You should be using .Parent.Range to qualify the Range method call with a specific worksheet, so that the active sheet isn't an implicit dependency.

But there's another bug: that -1 is assuming .Row is less than 10, i.e. that the row number in the .Address string is 1 single digit. If the user selects anything between rows 10 and 19, the workingArray will be 100 rows too tall; if the user selects anything between rows 20 and 29, it's 200 rows you don't need, that enter the working array, and become useless iterations here:

For i = LBound(workingArray, 1) To UBound(workingArray, 1)


It's not clear from just reading the code, but I also suspect the code assumes workingCell and targetCell are on the same row, on the same sheet:

With targetCell
End With


If the two user selections aren't on the same row, then the targetArray can be missing values: it would probably be a good idea to validate that, or make the procedure fail instead of producing wrong results; that could be caught and handled in the function responsible for collecting user input. If you know that an input value is already validated, you can Debug.Assert a precondition, and the code will only ever come to a halt if the assertion fails:

Debug.Assert Not workingCell Is Nothing
Debug.Assert Not targetCell Is Nothing
Debug.Assert workingCell.Parent = targetCell.Parent
Debug.Assert workingCell.Row = targetCell.Row


Seeing that at the top of a procedure that takes workingCell As Range and targetCell As Range parameters makes it pretty crystal-clear what the assumptions are, and allows the reader/maintainer to say "mismatching working and target rows? impossible!"

• I knew that Range would slip me up but I just couldn't see it until I tried deploying it 'in the wild'! I'll definitely have a look at the git features in Rubberduck, didn't know that was in there and need an excuse to brush up on git. Thanks for all the feedback, I'm probably going to have to re-read it a dozen times but I will implement everything I can. Oh and I also have Option Explicit by default for the very reason you mentioned, although it didn't save me where I used workingLastRow instead of comparisonLastRow in the comparisonArray block – Iain Saunders Sep 29 '16 at 11:16
• @IainSaunders note that v2.0 isn't quite stable yet; you can explore the features and git integration works pretty well, but do expect random crashes, especially upon closing or opening workbooks. v1.4.3 is more stable, but has a number of parser issues.. and git integration isn't quite working unfortunately. Feel free to watch github.com/rubberduck-vba/rubberduck for changes and releases, and star if you like! – Mathieu Guindon Sep 29 '16 at 15:15
• Thanks @Mat'sMug, I'm tracking my project now in GitHub and will be making the remaining changes soon. github.com/iain87/VBA_projects/tree/fixing/… – Iain Saunders Sep 29 '16 at 23:03
• Regarding this section: A function to collect user input; validates and returns a Range object. A function that takes a Range and returns an array. Would I need a function for the the second point? If I have the function MakeThisRange("A1",LastRow) then I imagine I could simply state myArray = MakeThisRange("A1",LastRow)? – Iain Saunders Oct 3 '16 at 13:30
• Yep, that would work =) – Mathieu Guindon Oct 3 '16 at 13:31

You should never duplicate code. Instead, code so similar you can almost cut-and-paste it is a code smell that you need to refactor. Now, VB is a sufficiently icky language that you can't always refactor well, but it seems like you want to take the code that occurs three times and turn it into a separate method. Since it modifies objects you later want to use, I would use ByRef parameters (note that these are, in fact, the default). Something like this ought to do:

Private Function readColumn&(ByRef arr)
Dim cell As Range
Set cell = Application.InputBox("Select the first cell of your comparison column.", Type:=8)
With cell
End With
End Function

Private Function writeColumnTarget(ByRef arr, ByVal lastRow&) As Range
Set writeColumnTarget = Application.InputBox("Select the first cell of your target column.", Type:=8)
With writeColumnTarget
End With
End Function


(You may want to further refactor this code; these methods use the exact same line to assign to the array.) Call these methods like so:

Dim workingLastRow&, workingArray

Then repeat such code, changing parameters and names as necessary. (I've used type characters to define Longs here; if you prefer a style without them, just replace the & ending function/sub/variable names with As Long.)
• Now, VB is a sufficiently icky language that you can't always refactor well - how is it any different than refactoring any other language? Are you mixing up VBA the language and the VBE IDE? It's the IDE that's under-featured as far as refactoring goes, the language itself has little to do with refactoring capabilities. Also I would strongly advise against using type hints over explicit type names. Type hints are a relic of dinosaurian BASIC where you declared DIM A% to save a few bits in your code because you had like 64Kb of RAM. Using them in modern code is good for obfuscation/golf. – Mathieu Guindon Sep 29 '16 at 15:02