Script loops over and over again - why?

Question

I am a VBA newbie and tried to make two loops that work with an If-Statement - as per this code:

Sub Filter()
For Count = Worksheets("STR").Range("A1").End(xlDown).Row To 1 Step -1
    For Counter = 5 To Worksheets("Filter").Range("A5").End(xlDown).Row
    With Worksheets("STR").Range("C" & Count)
        If .Value Like Worksheets("Filter").Range("C" & Counter).Value _
            Or .Value Like Worksheets("Filter").Range("D" & Counter).Value _
            Or .Value Like Worksheets("Filter").Range("E" & Counter).Value _
            Or .Value Like Worksheets("Filter").Range("A" & Counter).Value Then
                Worksheets("STR").Range("D" & Count) = "X"
                'Worksheets("STR").Row("A" & Count).EntireRow.Delete
            Next Counter
        End If
    End With
Next Count
End Sub

At some stage I suppose it loops over and over again, until it crashes. The debugger tells me it's the "End If" Line, but I don't see what's wrong. Is there a way to make this easier?

Thanks in advance!

As presented the code doesn't run - you would get a Next without For error because the `Next Counter` line should be after the `End If` and `End With` and not before them — barrowc, Aug 13 '12 at 21:19
Hey Barrowc, thx for that tip - but it still doesn't work (or takes REALLY long). Is there any way to make this perform better/at all? — oliver13, Aug 13 '12 at 21:31

score 2 · Accepted Answer · edited Jun 20 '20 at 09:12

Four things

Indent your code. That will ensure that you don't make errors as @barrowc suggested.
I would suggest not to use End(xlDown) to find the last row. See this link on how to find the last row.
Turn ScreenUpdating to False and set it back on.
Use Error handling

Your code after applying the above 4 suggestions will look like this.

Option Explicit

Sub Filter()
    Dim LastRowSTR As Long, LastRowFilter As Long
    
    On Error GoTo Whoa
    
    Application.ScreenUpdating = False
    
    With Sheets("STR")
        LastRowSTR = .Range("A" & .Rows.Count).End(xlUp).Row
    End With

    With Sheets("Filter")
        LastRowFilter = .Range("A" & .Rows.Count).End(xlUp).Row
    End With
    
    For Count = LastRowSTR To 1 Step -1
        For Counter = 5 To LastRowFilter
            With Worksheets("STR").Range("C" & Count)
                If .Value Like Worksheets("Filter").Range("C" & Counter).Value _
                Or .Value Like Worksheets("Filter").Range("D" & Counter).Value _
                Or .Value Like Worksheets("Filter").Range("E" & Counter).Value _
                Or .Value Like Worksheets("Filter").Range("A" & Counter).Value Then
                    Worksheets("STR").Range("D" & Count) = "X"
                    'Worksheets("STR").Row("A" & Count).EntireRow.Delete
                End If
            End With
        Next Counter
    Next Count
LetsContinue:
    Application.ScreenUpdating = True
    Exit Sub
Whoa:
    MsgBox Err.Description
    Resume LetsContinue
End Sub

INTERESTING READ.

TOPIC: To ‘Err’ is Human

LINK: http://siddharthrout.wordpress.com/2011/08/01/to-err-is-human/

+1 might want to throw in an `Option Explicit` at the top of the module if there isn't one there already — barrowc, Aug 13 '12 at 21:38
...and the `Option Explicit` means that `Count` and `Counter` need to be declared (unless they are in scope from somewhere else) — barrowc, Aug 13 '12 at 22:01

Script loops over and over again - why?

1 Answers1