Peter, as always, yours worked perfectly. AND I was actually able to follow what it was doing.
Thank you!
You are very welcome.
so, would you tell me if you have any notes on this
Since you asked, I do.
1. Since we are looking to match or not match
exact locations, not searching for text within longer strings, regular expressions are an inefficient way to approach the problem. See also alternative code below.
2. Your code interacts with the worksheet many times - each cell value is retrieved from the worksheet one at a time and whenever there is a row deletion to do your code interacts with the worksheet again. Such interactions between code and worksheet are relatively slow. For example, your last code is about 20 times slower than the one I posted earlier and one of the reasons for the difference is those constant interactions.
3. Your red code below is a somewhat awkward way to achieve the row deletion when required. A much simpler way would be the single blue code line.
Rich (BB code):
If .test(Cells(j, 6)) Then
Else
Cells(j, 6).EntireRow.Delete
End If
If Not .test(Cells(j, 6)) Then Rows(j).Delete
4. You have declared
some of your variables with Dim statements, but not
all. I would highly recommend declaring
all variables. You can easily force yourself to do that by, in the vba window, Tools -> Options -> Editor tab -> Require Variable Declaration -> OK
5. Your code relies on Sheet1 being the active sheet when the code is run. It would be more robust if you didn't have to rely on that being the case.
6. Your code still does not not work correctly. For example, with the location values shown below, South London and Londonderry should be removed from Sheet1 but your code does not remove them because they contain the text "London". This relates to my point 1 above.
Sheet1
London
South London
Londonderry
New York
Sheet 2
London
New York
IF we were to approach the problem by looping through the rows, & I'm not suggesting that approach at all, then here is a way that addresses points 1, 5 & 6 and uses vba's built-in string functions which should be more efficient than using RegExp.
Rich (BB code):
Sub testagain()
Dim r As Long
Dim List As String
Application.ScreenUpdating = False
With Sheets("Sheet2")
List = "|" & Join(Application.Transpose(.Range("A2", .Range("A" & .Rows.Count).End(xlUp))), "|") & "|"
End With
With Sheets("Sheet1")
For r = .Cells(.Rows.Count, 1).End(xlUp).Row To 2 Step -1
If InStr(1, List, "|" & .Cells(r, 6).Value & "|", vbTextCompare) = 0 Then .Rows(r).Delete
Next r
End With
Application.ScreenUpdating = True
End Sub
Finally, if speed
was an issue, and it shouldn't be for the thousand or so rows for this OP, here is a very fast way to do the deletions - about twice as fast as the code I posted in post 9. This code only interacts with the sheets 5 times:
- once to get the list from Sheet2
- once to get all the column F values from Sheet1
- once to write a column of values back to Sheet1
- once to sort
- once to do all the row deletions at once.
Rich (BB code):
Sub testfast()
Dim r As Long, k As Long
Dim List As String
Dim a As Variant, b As Variant
With Sheets("Sheet2")
List = "|" & Join(Application.Transpose(.Range("A2", .Range("A" & .Rows.Count).End(xlUp))), "|") & "|"
End With
With Sheets("Sheet1")
With .Range("A2:I" & .Range("A" & .Rows.Count).End(xlUp).Row)
a = .Columns(6).Value
ReDim b(1 To UBound(a), 1 To 1)
For r = 1 To UBound(a)
If InStr(1, List, "|" & a(r, 1) & "|", vbTextCompare) = 0 Then
b(r, 1) = 1
k = k + 1
End If
Next r
If k > 0 Then
Application.ScreenUpdating = False
.Columns(.Columns.Count).Value = b
.Sort Key1:=.Columns(.Columns.Count), Order1:=xlAscending, Header:=xlNo
.Resize(k).EntireRow.Delete
Application.ScreenUpdating = True
End If
End With
End With
End Sub
