trjhtr

I'm working on a large VBA project for work (around 1000+ lines of vba code) This module is called within another module where I disable everything possible like screen updating, events and setting calculation to manual to improve speed, yet this still seems to run slowly when I'm working with sheets with over 60,000 rows.

What this code does, is filter a row in one sheet and copy paste it into another sheet. Then use the data from this to vlookup onto another sheet. I don't understand the syntax of index match, otherwise I could have eliminated the cut/paste option to move the column on the right for the vlookup to speed it up.

I suspect a few other elements in this code chunk are slowing it down too. Could you please help me identify what is causing the issue? Is it the autofilter?

Sub AdgroupName()

Dim LrowBlk As Long
LrowBlk = Sheets("Bulk Sheet").Cells(Rows.Count, "H").End(xlUp).Row
Sheets("Bulk Sheet").AutoFilterMode = False
Sheets("Bulk Sheet").Range("H3:I3").AutoFilter field:=2, Criteria1:="AdGroup"
ActiveWorkbook.Sheets("Bulk Sheet").Range("H3:K3" & LrowBlk).SpecialCells(xlCellTypeVisible).Copy
Worksheets("Adgroup List").Paste Destination:=Worksheets("Adgroup List").Range("A1")
ActiveWorkbook.Sheets("Bulk Sheet").Range("P3:P" & LrowBlk).SpecialCells(xlCellTypeVisible).Copy
Worksheets("Adgroup List").Paste Destination:=Worksheets("Adgroup List").Range("E1")
With Sheets("Adgroup List")
 .Columns("C:C").Cut
 .Columns("E:E").Insert Shift:=xlToRight
 End With
Application.CutCopyMode = False
Dim ws As Worksheet
Dim LrowSTR As Long
Set ws = ThisWorkbook.Sheets("Search Term Report")
LrowSTR = Sheets("Search Term Report").Cells(Rows.Count, "H").End(xlUp).Row
Dim Strformulas(1 To 3) As Variant

With ws
 Strformulas(1) = "=VLOOKUP($K4,'Adgroup List'!$C:$D,2,0)"
 Strformulas(2) = "=VLOOKUP($K4,'Adgroup List'!$C:$E,3,0)"
 .Range("AJ4:AK4").Formula = Strformulas
 .Range("AJ4:AK" & LrowSTR).FillDown
 .Range("AJ3").Value = "Campaign ID"
 .Range("AK3").Value = "AdGroup Name"
 End With
 Sheets("Bulk Sheet").AutoFilterMode = False
 Sheets("Adgroup List").AutoFilterMode = False
 End Sub

ooh wee Because of the below statement, I'll do this in little steps which may not be the best approach, but should be easy to follow.

I don't understand the syntax of index match, otherwise I could have eliminated ...

_{let me drop an article from my website right here}

I'm not bashing you for not knowing index/match, but you might as well learn it! ^{use it to look up to the left}

VBA and worksheet functions

All right, so you're using VBA - you don't need to use worksheet functions at all. If you need the function to exist because the audience needs to see the functions rather than the results, then that's another issue.

ActiveWorkbook.Sheets("Bulk Sheet").Range("H3:K3" & LrowBlk).SpecialCells(xlCellTypeVisible).Copy

Is this essentially .Range("H3:K3" 130) (using 130 arbitrarily)? How does that work?

You also sort based on H3:I3 but then copy H3:K3 and P - are columns J through P filtered when you filter based on column I?

That's pretty confusing. Essentially you have a range (like "H3:P400") that you want to get elements from only if columns 2 matches a string ("adGroup")?

So what? Arrays.

There are a lot of ways to do this, but let's start with the way you've done it, but instead use an array -

bulksheet.Range("H3:P" & lastRow).AutoFilter field:=2, Crteria1:="AdGroup"
Dim bulkData As Variant
bulkData = bulksheet.Range("H3:K" & lastRow).SpecialCells(xlCellTypeVisible)

Right now you have all your data in a variant where you can do your thing instead of on the sheet. Essentially, it's the same advice as

Be sure to avoid things like .Select - it just slows the code down
by needing to fiddle with the spreadsheet while doing everything else
behind the scenes. There's a good question on StackOverflow
addressing
this.

So now you have a variant where there are 9 columns and you need 4 of them on a new sheet. You could just put them on the new sheet like this -

For i = LBound(bulkData) To UBound(bulkData)
 adGroupSheet.Cells(i, 1) = bulkData(i, 1)
 adGroupSheet.Cells(i, 2) = bulkData(i, 2)
 adGroupSheet.Cells(i, 3) = bulkData(i, 3)
 adGroupSheet.Cells(i, 1) = bulkData(i, 9) 
Next

But maybe there's a better way to do all of that (I'm not 100%, I might have read somewhere that autofilter is faster than a loop, but I digress). Simply -

Dim bulkData As Variant
Dim bulkLastRow As Long
bulkLastRow = bulksheet.Cells(bulksheet.Rows.Count, 8).End(xlUp).Row
bulkData = bulksheet.Range(bulksheet.Cells(1, 8), bulksheet.Cells(lastrow, 16))
Dim sortedData As Variant
ReDim sortedData(1 To bulkLastRow, 1 To 4)
Dim bulkIndex As Long
Dim sortedIndex As Long
sortedIndex = 1
For bulkIndex = LBound(bulkData) To UBound(bulkData)
 If bulkData(bullkindex, 2) = "AdGroup" Then
 sortedData(sortedIndex, 1) = bulkData(bulkIndex, 1)
 sortedData(sortedIndex, 2) = bulkData(bulkIndex, 2)
 sortedData(sortedIndex, 3) = bulkData(bulkIndex, 3)
 sortedData(sortedIndex, 4) = bulkData(bulkIndex, 9)
 sortedIndex = sortedIndex + 1
 End If
Next

Right, now you have your sorted data in a variant and could easily just

 sortedsheet.Range(sortedsheet.Cells(1, 1), sortedsheet(sortedIndex, 4)) = sortedData

Now you can write your formulas.

But wait

But, I see now that you moved column 3 to column 4 -

With Sheets("Adgroup List")
 .Columns("C:C").Cut
 .Columns("E:E").Insert Shift:=xlToRight
 End With

This could easily be fixed by just putting it there in the first place with something like

 ReDim sortedData(1 To bulkLastRow, 1 To 5)
 sortedData(sortedIndex, 1) = bulkData(bulkIndex, 1)
 sortedData(sortedIndex, 2) = bulkData(bulkIndex, 2)
 sortedData(sortedIndex, 4) = bulkData(bulkIndex, 3)
 sortedData(sortedIndex, 5) = bulkData(bulkIndex, 9)

And you have a blank column.

All right, so?

But, why stop there? Assuming you don't need the formulas to be present on the sheet, you can get your data from your variant. But let's examine what you're doing first -

LrowSTR = termSheet.Cells(termSheet.Rows.Count, 8).End(xlUp).Row
Dim termData As Variant
ReDim termData(1 To 3)
termData(1) = "=VLOOKUP($K4,'Adgroup List'!$C:$D,2,0)"
termData(2) = "=VLOOKUP($K4,'Adgroup List'!$C:$E,3,0)"

You make a 1 dimensional array, look up a single value to populate 2 of the 3 items and never use your LrowSTR. Then

.Range("AJ4:AK4").Formula = Strformulas
.Range("AJ4:AK" & LrowSTR).FillDown

Yikes - of course that will hang up. Even without really changing your code it would be easier to just

For i = LBound(termData) To UBound(termData)
 termData(i, 1) = "=VLOOKUP($K4,'Adgroup List'!$C:$D,2,0)"
 termData(i, 2) = "=VLOOKUP($K&" & i + 3 & ",'Adgroup List'!$C:$E,3,0)"
Next

And then paste it all at once, no need to autofill (which is definitely slow). You're also searching column "C" when you already moved it and it's empty. Why not just look in D and E? I digress

Regardless, you might just

Dim termData As Variant
termData = termsheet.Range(termsheet.Cells(4, 11), termsheet(LrowSTR, 11))
ReDim Preserve termData(1 To UBound(termData), 1 To 3)
'find things in sortedData and populate termData with values
'print to termSheet

Misc

Other things to note -

1. You've declared all of your variables, that's great! Your naming could be improved a bit and Standard VBA naming conventions have camelCase for local variables and PascalCase for other variables and names. So LrowBlk would be bulkLastRow or something. And ws would be a terrible name, but




2. Worksheets have a CodeName property - View Properties window (F4) and the (Name) field (the one at the top) can be used as the worksheet name. This way you can avoid Sheets("mySheet") and instead just use mySheet.




3. Always turn on Option Explicit. You can have it automatically by going to Tools -> Options in the VBE and checking the Require Variable Declaration option. This way if you have any variables not defined, the compiler will let you know. This isn't a problem for you, but it's good practice - I have it on.




4. It's good practice to indent all of your code that way Labels will stick out as obvious. VBE standard is 4 spaces between levels