VBA Text Array - Scan two columns rather than one I have some code which is designed to scan columns F and G for occurrences of words found in an array, the array containing text found in column J.

Column J contains free text from a field in SAP. Being free text, it could be "Kerry John Pub Expenses" or "CATS O/H Kerry John", or even "CATS John Kerry O/H". There is no data entry standard for this field... and this is what makes this task difficult.

Example

Here we have 4 rows of data, "John Citizen" is located in row 3, therefore the blank cells in columns F and G, row 2 can be populated with his first and last name.





I would like some suggestions as to how this code could be re-written to achieve this result more efficiently

    Sub arraycolumnmatch()
    Dim txtArray As Variant, T As Variant
    Dim I As Long, J As Long

    For I = 2 To Range("E50000").End(xlUp).row
        typ = Range("F" &amp; I).Value
    If typ = "" Then
        txt = Range("J" &amp; I).Value
        txtArray = Split(txt, " ")

        For Each T In txtArray
            For J = 2 To Range("G50000").End(xlUp).row
                If Range("G" &amp; J).Value = T Then
                    match_txt = T
                    Range("G" &amp; I).Value = match_txt
                    Exit For
                End If
            Next J
        Next T

        For Each T In txtArray
            For J = 2 To Range("F50000").End(xlUp).row
                If Range("F" &amp; J).Value = T Then
                       match_txt = T

                 'Check for surnames which could match first names such as Mary Michael
                       If Not Range("G" &amp; I).Value = T Then

                          Range("F" &amp; I).Value = match_txt
                          Exit For
                       End If
                End If
            Next J
        Next T
   End If
Next I
End Sub
Consistent Indentation.
The first thing I would address, is the indentation. Improving code that isn't properly formatted is just... not right.
Here are the signs:
Sub arraycolumnmatch()
Dim txtArray As Variant, T As Variant
Dim I As Long, J As Long

And:
Next I
End Sub

But also:
 'Check for surnames which could match first names such as Mary Michael
       If Not Range(&quot;G&quot; &amp; I).Value = T Then

The key is consistency - here's your code, with consistent indentation (comments omitted). Notice each code block (If...End If, For...Next, etc.) consistently adds an indentation level, and the end of the block lines up with its beginning - also notice that indentation levels are consistently 4 spaces wide; that's the default VBE setting for the Tab key:
Sub arraycolumnmatch()
|
|   Dim txtArray As Variant, T As Variant
|   Dim I As Long, J As Long
|
|   For I = 2 To Range(&quot;E50000&quot;).End(xlUp).row
|   |   typ = Range(&quot;F&quot; &amp; I).Value
|   |   If typ = &quot;&quot; Then
|   |   |   txt = Range(&quot;J&quot; &amp; I).Value
|   |   |   txtArray = Split(txt, &quot; &quot;)
|   |   |
|   |   |   For Each T In txtArray
|   |   |   |   For J = 2 To Range(&quot;G50000&quot;).End(xlUp).row
|   |   |   |   |   If Range(&quot;G&quot; &amp; J).Value = T Then
|   |   |   |   |   |   match_txt = T
|   |   |   |   |   |   Range(&quot;G&quot; &amp; I).Value = match_txt
|   |   |   |   |   |   Exit For
|   |   |   |   |   End If
|   |   |   |   Next J
|   |   |   Next T
|   |   |
|   |   |   For Each T In txtArray
|   |   |   |   For J = 2 To Range(&quot;F50000&quot;).End(xlUp).row
|   |   |   |   |   If Range(&quot;F&quot; &amp; J).Value = T Then
|   |   |   |   |   |   match_txt = T
|   |   |   |   |   |   If Not Range(&quot;G&quot; &amp; I).Value = T Then
|   |   |   |   |   |   |   Range(&quot;F&quot; &amp; I).Value = match_txt
|   |   |   |   |   |   |   Exit For
|   |   |   |   |   |   End If
|   |   |   |   |   End If
|   |   |   |   Next J
|   |   |   Next T
|   |   End If
|   Next I
|
End Sub

Much easier to follow, isn't it?

Meaningful Names.
Next thing I would address is the naming. Improving code where you have to quadruple-check every change you make because you're not sure you're changing the right thing is just not efficient. Using meaningful names for all identifiers fixes that.
Starting with the method's name itself:
Sub arraycolumnmatch()

Typically, VBA (VB in general) method names are PascalCase, for better readability. That would make it ArrayColumnMatch - but good method/procedure names should also start with a verb. What's this method doing exactly? should be answerable by simply looking at it's name.
The problem is, the method in question is doing too many things, so giving it a meaningful name is hard. But I'll get back to that.
Avoid chopping off identifers, like typ when you meant type - but then Type is a reserved keyword.. the solution isn't to make it less readable! I'm not sure which worksheet Range is referring to because Range is an implicit reference to Application.ActiveSheet (and that's very bug-prone!), so I'll just assume you meant to call it documentType.
txt is another meaningless name (heck, people use that as a prefix for TextBox controls!): it stands for that manual entry field you're trying to parse, right? How about targetFieldValue, or manualTextField?
Also avoid single-letter identifiers, like T - assuming txt is manualTextField, I'd go with manualTextFieldValue.
Local variables are typically named in camelCase, so I and J would be i and j - those are typically used for iterating loops as you're doing, so I'm not going to complain about those, except both loops seem to be iterating the very same rows on the active sheet (whatever that is), and they're nested, too... and since they're row numbers I'd typically name them xlRow[WhatItsFor].

Undeclared Identifiers
Always, systematically, consistently, automatically, thoughtlessly stick Option Explicit at the top of every VBA module. Don't question it, just do it: without it, you can have a bug by simply having a little typo in an identifier name. With it, your VBA code will refuse to compile and run if an identifier isn't declared anywhere. Use it. Always.
If match_txt is declared outside the scope of the procedure, then move the declaration inside that scope - it doesn't need to be at module scope. Always declare variables at the smallest possible scope; globals are evil.

vbNullString
A very minor point, but I have to mention it.
If typ = &quot;&quot; Then

Avoid using an empty string &quot;&quot; to mean no value. VBA defines the constant vbNullString specifically for that. And why should you bother typing vbNullString when &quot;&quot; does the same thing?
That's the thing. It doesn't do the same thing. &quot;&quot; is a full-fledged String value that needs its own allocated memory space. vbNullString is a null string pointer that's not allocated anywhere. Of course it's a very minor performance hit to allocate that string, but semantically, vbNullString is the right thing to do.
Don't believe me? Try this in the immediate pane (Ctrl+G):
?&quot;Address for empty string: &quot; &amp; StrPtr(&quot;&quot;) &amp; vbNewLine &amp; &quot;Address for vbNullString: &quot; &amp; StrPtr(vbNullString)

I get this output:
Address for empty string: 241665720
Address for vbNullString: 0

There's some kind of optimization going on under the hood though, so the pointer for &quot;&quot; will be reused every time it's encountered in your application's lifetime - as I said it's a very minor point. But just as you wouldn't declare an unused variable, you shouldn't allocate a memory address for a built-in constant value.

Twin Loops
You have this:
For Each T In txtArray

And then a few lines further, you have that:
For Each T In txtArray

You're iterating all elements of txtArray twice, when once would suffice. Also, wouldn't the last row of column &quot;G&quot; be the same last row as in column &quot;F&quot;? Wouldn't that last row be the same for every iteration of T? I'm asking, because your data seems to be laid out in a table with records, so you'd only need to find the row for the last record, whichever column you're using for that is irrelevant, no?
For J = 2 To Range(&quot;G50000&quot;).End(xlUp).row
...
For J = 2 To Range(&quot;F50000&quot;).End(xlUp).row

I'd extract a variable here, and assign it to Range(&quot;G50000&quot;).End(xlUp).row before the For Each T In txtArray loop starts, and then do
For J = 2 To xlLastRow

Then you're writing to G[i] if the value of G[j] matches the current txtArray element, and to F[i] if G[i] wasn't written to and the value of F[j] matches the current txtArray element.
Wouldn't that do the same thing then?
If Range(&quot;G&quot; &amp; j).Value = T Then
    Range(&quot;G&quot; &amp; i).Value = T
Else If Range(&quot;F&quot; &amp; j).Value = T Then
    Range(&quot;F&quot; &amp; i).Value = T
End If

Lastly, the J loop looks like it could be implemented as a Find instead of a loop.

Extract Methods.

The problem is, the method in question is doing too many things, so giving it a meaningful name is hard. But I'll get back to that.

So here we are. What's that method doing actually?
It...

finds the row number for the last record on the active sheet
iterates all records on the active sheet
splits a value in the current record into an array of values
iterates all values in that array
looks for a match of that value somewhere in columns G and F
writes that value to column G or F, whichever comes first

Extracting methods is a refactoring operation, and doing that manually in VBA is, honestly, a pain in the neck.
I've authored a refactoring tool that does exactly that (and more):

Of course I have a little bit of a vested interest (very little - it's free and open-source actually), but I'd recommend you take a look at what the Rubberduck VBE add-in can do for you.