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" & I).Value
    If typ = "" Then
        txt = Range("J" & I).Value
        txtArray = Split(txt, " ")

        For Each T In txtArray
            For J = 2 To Range("G50000").End(xlUp).row
                If Range("G" & J).Value = T Then
                    match_txt = T
                    Range("G" & 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" & J).Value = T Then
                       match_txt = T

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

                          Range("F" & 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("G" & 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("E50000").End(xlUp).row
|   |   typ = Range("F" & I).Value
|   |   If typ = "" Then
|   |   |   txt = Range("J" & I).Value
|   |   |   txtArray = Split(txt, " ")
|   |   |
|   |   |   For Each T In txtArray
|   |   |   |   For J = 2 To Range("G50000").End(xlUp).row
|   |   |   |   |   If Range("G" & J).Value = T Then
|   |   |   |   |   |   match_txt = T
|   |   |   |   |   |   Range("G" & 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" & J).Value = T Then
|   |   |   |   |   |   match_txt = T
|   |   |   |   |   |   If Not Range("G" & I).Value = T Then
|   |   |   |   |   |   |   Range("F" & 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 = "" Then

Avoid using an empty string "" to mean no value. VBA defines the constant vbNullString specifically for that. And why should you bother typing vbNullString when "" does the same thing?

That's the thing. It doesn't do the same thing. "" 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 righ

StackExchange Code Review Q#87503, answer score: 8