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 -
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
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.
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.
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
