Assigning value to cells with Excel VBA

Question

I have an Excel Worksheet consisting of two columns, one of which is filled with strings and the other of which is empty. I would like to use VBA to assign the value of the cells in the empty column based on the value of the adjacent string in the other column.

Dim regexAdmin As Object 
Set regexAdmin = CreateObject("VBScript.RegExp") 
regexAdmin.IgnoreCase = True
regexAdmin.Pattern = "Admin" 
Dim i As Integer
For i = 1 To 10 'let's say there is 10 rows
 Dim j As Integer
 For j = 1 To 2
 If regexAdmin.test(Cells(i, j).Value) Then
 Cells(i, j + 1).Value = "Exploitation"
 End If
 Next j
Next i

The problem is that when using this loop for a big amount of data, it takes way too long to work and, most of the time, it simply crashes Excel.

Does anyone know a better way of doing this?

Answer 1

The short answer is:

Don't use vba, use a formula. In particular, a combination of IF and SEARCH.

=IF(SEARCH($A1,"Admin")>0,"Exploitation","")

But this is code review, so let's do that anyway.

• Regex is slow. It seems that you're only using it for it's case insensitivity. Given that, you can directly compare cell values by using StrComp with the vbTextCompare option. (useful article on StrComp)

• i and j are typically used for loop counters, but row and col make more sense in this case.

Here is what this could could look like:

Dim row As Integer
For row = 1 To 10 'let's say there is 10 rows
 Dim col As Integer
 For col = 1 To 2
 If StrComp("Admin",Cells(row, col).Value,vbTextCompare) Then
 Cells(row, col + 1).Value = "Exploitation"
 End If
 Next col
Next row

Answer 2

I would think that simple string comparison would be much faster than Regex.

Dim pattern as string
pattern = "Admin"
Dim i As Integer
For i = 1 To 10 'let's say there is 10 rows
 Dim j As Integer
 For j = 1 To 2
 If Cells(i, j) = pattern Then
 Cells(i, j + 1) = "Exploitation"
 End If
 Next j
Next i

• RubberDuck: This code doesn't address case insensitive comparison, but you're correct. The regex is overkill and string comparison is preferred in this case. (Welcome to Code Review by the way!)

Answer 3

In B1:

=if(upper(A1)="ADMIN","Exploitation","")

Then just fill it down. This is case insensitive.

This auto fill can be done two ways, either interactively in the worksheet, or programmatically:

Interactively: Excel has an autofill feature. With B1 selected and that formula put in, just double click the fill handle, which is the tiny square in the bottom right corner of cell when it's selected. Excel will intelligently copy the formula down to the end of the contiguous range that has data. Meaning if A1-A256 has data with no blanks, it'll autofill to B256. Alternatively, if there are blanks, scroll to the bottom and select B256 (or whatever the end is). Then Ctrl + Shift + Up arrow to select the range leading to B1, and Ctrl+D to copy it down (think d= ditto)

Using VBA... if you must do this programmatically: with the formula containing cell as your selection:

Selection.AutoFill Destination:=Range("B1:B19")

There's other options available for autofill to do a few cool tricks. Can copy a literal value instead of a formula, or also fill a series based on a pattern. You can also set custom patterns for it to recognize, such as lines of business you commonly repeat in stuff or cities you have retail locations in, etc.

• RubberDuck: Welcome to Code Review indeed! I completely missed that OP didn't need to search the column! ++ Best solution here.

Answer 4

Wow. Just reading through the first couple of lines got me wondering:

• Why the late-binding?
• Why use a regex at all?

@ckuhn203 already addressed the naming in his answer, but I find this:

Dim i As Integer
For i = 1 To 10 'let's say there is 10 rows

Turned into that:

Dim row As Integer
For row = 1 To 10 'let's say there is 10 rows

...Doesn't need the comment anymore.

---

I would like to use VBA to assign the value of the cells in the empty column based on the value of the adjacent string in the other column.

I think that's [mis|ab]using VBA: Excel itself is very good at dealing with assigning cell values based on other cells' values.

---

regexAdmin.Pattern = "Admin" 

I think that's [mis|ab]using regex: if your pattern is just a plain word, you're most probably trying to kill a mosquito with a bazooka. Wrong tool for the job here.

Answer 5

May I suggest a 50% reduction in runtime/effort?

Dim row As Integer
For row = 1 To 10 'let's say there is 10 rows
 If StrComp("Admin",Cells(row, 1).Value,vbTextCompare) Then
 Cells(row, 2).Value = "Exploitation"
 End If
Next row

Did nobody notice that the OP talks about "checking ONE column, writing to the NEXT adjacent", really? Why loop columns then? The second pass would only check either an empty cell or one with "Exploitation" in it.

Answer 6

Whenever you are accessing the Range object it should be done with a single read/write operation.

Prior to entering the for loop you should read the entire range that you are looking to work with.

data = Range(Cells(1,1), Cells(10,2)).Value

Now you can work with the data:

For i = 1 To 10 'let's say there is 10 rows
 Dim j As Integer
 For j = 1 To 2
 If regexAdmin.test(data(i, j)) Then
 data(i, j + 1) = "Exploitation"
 End If
 Next j
Next i

Finally write the data back to excel:

Range(Cells(1,1), Cells(10,2)).Value = data

Answer 7

Combining all the other answers together, made it case insensitive like the regex in the original, removed the need to specify how many rows and declaring all the variables because Option Explicit avoids so many errors in vba

Option Explicit
Sub checkForExploit()
Dim row As Integer
Dim data() As Variant
Dim datarange As Range
Set datarange = Range("A1:B10")
data = datarange.Value
For row = 1 To UBound(data, 1)
 If LCase(data(row, 1)) = "admin" Then
 data(row, 2) = "Exploitation"
 End If
Next row
datarange.Value = data
End Sub