Extracting Country Names from Cell Values

This fairly clunky looking VBA script successfully pulls names of countries from an Array, but could it be made shorter?

Mass of text from Column C onward in each row may contain the names of countries, which I want to pull into the corresponding cell in row A. What I have so far is not pretty...

```
Sub PullCountries()
Dim Rng As Range
Dim Block As Range
Dim i As Long
i = 1

Dim LastRow As Long
LastRow = Range("B1").End(xlDown).Row

While i <= LastRow
Set Rng = Range("B" & i)
Set Block = Rows(i)

Range("I1").Value = i

If InStr(1, Range("C" & i), "Canada") Or InStr(1, Range("D" & i), "Canada") Or InStr(1, Range("E" & i), "Canada") Then
Rng.Offset(0, -1).Value = "Canada"
i = i + 1
ElseIf InStr(1, Range("C" & i), "United States") Or InStr(1, Range("D" & i), "United States") Or InStr(1, Range("E" & i), "United States") Then
Rng.Offset(0, -1).Value = "United States"
i = i + 1
ElseIf InStr(1, Range("C" & i), "Britian") Or InStr(1, Range("D" & i), "Britian") Or InStr(1, Range("E" & i), "Britian") Then
Rng.Offset(0, -1).Value = "UK"
i = i + 1
ElseIf InStr(1, Range("C" & i), "UK") Or InStr(1, Range("D" & i), "UK") Or InStr(1, Range("E" & i), "UK") Then
Rng.Offset(0, -1).Value = "UK"
i = i + 1
ElseIf InStr(1, Range("C" & i), "Spain") Or InStr(1, Range("D" & i), "Spain") Or InStr(1, Range("E" & i), "Spain") Then
Rng.Offset(0, -1).Value = "Spain"
i = i + 1
ElseIf InStr(1, Range("C" & i), "Portugal") Or InStr(1, Range("D" & i), "Portugal") Or InStr(1, Range("E" & i), "Portugal") Then
Rng.Offset(0, -1).Value = "Portugal"
i = i + 1
ElseIf InStr(1, Range("C" & i), "Ireland") Or InStr(1, Range("D" & i), "Ireland") Or InStr(1, Range("E" & i), "Ireland") Then
Rng.Offset(0, -1).Value = "Ireland"
i = i + 1
ElseIf InStr(1, Range("C" & i), "Japan") Or InStr(1, Range("D" & i), "Japan") Or InStr(1, Range("E" & i), "Japan") Then
Rng.Offset(0, -1).Value = "Japan"
i = i + 1
ElseIf InStr(1, Range("C" & i), "Greece") Or InStr(1, Range("D" & i), "Greece") Or InStr(1, Range("E" & i), "Greece")

The small(er) things before we tackle the elephant in the room:

Naming

Your names aren't terrible, but could be a lot better. However, whatever you name things, you should comply with standard naming conventions. To wit:


Local Variables: Written in camelCase.


Dim localVariable As String

includes method arguments.


Module / Global Variables: Written in PascalCase.


Private ModuleVariable As String

Global PublicVariable As Long


Method Names: Verbs. Written in PascalCase


Private Function ReturnThisValue() As Long

Public Sub DoThisThing()


Constants: Written in SHOUTY_SNAKE_CASE


Public Const CONSTANT_VALUE As String = "This Value Never Changes"

Also, Block is declared and set but never used. It should be removed.

Don't use :

Just don't. Keep your instructions on separate lines. They're too easy to miss and violate too many conventions.

Put things in Variables

Range("C" & i)

You see this? This is telling Excel to go and find that range. Every time you write it. What if you want to check a different column? Right now, you'll have to rewrite the declaration on 20 different lines.

Instead, put it in a variable then just reference the Variable. Now, if the variables need to change, you only have to change them in 1 place, and the rest takes care of itself.

Dim cCell As Range, dCell As Range, eCell As Range
Set cCell = Range("C" & i)
Set dCell = Range("D" & i)
Set eCell = Range("E" & i)

Dim countryName As String

countryName = "Canada"
If Instr(1, cCell, countryName) Or Instr(1, dCell, countryName) Or Instr(1, eCell, countryName) Then
...
...

Don't Repeat Yourself

Also known as DRY. Take your i = i + 1 statement. That is always going to happen. So why write it 20 times when you can just put it at the start or end of your loop?

While i <= lastRow

    Code
    Code
    Code

    ...

    i = i + 1

Wend

Boom. 12 lines of code gone

And now the big stuff:

Refactoring

Refactoring is the process of splitting one big thing into many little things.
Any time you find yourself copy-pasting code, you should be thinking "Hmm, this can probably be turned into a method of some kind".

1st Refactoring

This check:

If Instr(1, cCell, countryName) Or Instr(1, dCell, countryName) Or Instr(1, eCell, countryName) Then

Can be a Separate Method Like so:

Public Function NameIsInRange(ByVal searchName As String, ByRef range1 As Range, range2 As Range, range3 As Range) As Boolean

    Dim result As Boolean
    result = InStr(1, range1, searchName) Or InStr(1, range2, searchName) Or InStr(1, range3, searchName)

    NameIsInRange = result

End Function

And now we're down to:

Sub PullCountries()

    Dim i As Long
    i = 1

    Dim LastRow As Long
    LastRow = Range("B1").End(xlDown).Row

    While i <= LastRow

        Dim resultRange As Range
        Set resultRange = Range("A" & i)

        Dim cCell As Range, dCell As Range, eCell As Range
        Set cCell = Range("C" & i)
        Set dCell = Range("D" & i)
        Set eCell = Range("E" & i)

        If NameIsInRange("Canada", cCell, dCell, eCell) Then
        resultRange = "Canada"

        ElseIf NameIsInRange("United States", cCell, dCell, eCell) Then
        resultRange = "United States"

        ElseIf NameIsInRange("Britian", cCell, dCell, eCell) Then
        resultRange = "UK"

        ElseIf NameIsInRange("UK", cCell, dCell, eCell) Then
        resultRange = "UK"

        ElseIf NameIsInRange("Spain", cCell, dCell, eCell) Then
        resultRange = "Spain"

        ElseIf NameIsInRange("Portugal", cCell, dCell, eCell) Then
        resultRange = "Portugal"

        ElseIf NameIsInRange("Ireland", cCell, dCell, eCell) Then
        resultRange = "Ireland"

        ElseIf NameIsInRange("Japan", cCell, dCell, eCell) Then
        resultRange = "Japan"

        ElseIf NameIsInRange("Greece", cCell, dCell, eCell) Then
        resultRange = "Greece"

        ElseIf NameIsInRange("Italy", cCell, dCell, eCell) Then
        resultRange = "Italy"

        End If

        i = i + 1

    Wend

End Sub

Public Function NameIsInRange(ByVal searchName As String, ByRef range1 As Range, range2 As Range, range3 As Range) As Boolean

    Dim result As Boolean
    result = InStr(1, range1, searchName) Or InStr(1, range2, searchName) Or InStr(1, range3, searchName)

    NameIsInRange = result

End Function

2nd Refactoring

The only things that actually change in the loop are the name to check and the name to output. So, why don't we make those a list? For an iterable list with more than 1 element per line, I'd use an Array.

Let's make a new sheet and give it the codename wsCountryNames. Then a function to get the table and pass it to an Array:

```
Public Function GetCountryNamesTable() As Variant

With wsCountryNames

Dim topLeftCell As Range
Set topLeftCell = .Cells(1, 1) '/ "A1"

Dim finalRow As Long

StackExchange Code Review Q#124462, answer score: 10