CODEWITHSUNDEEP
excelvba
ARL
ARL

Reputation: 64

Please help me understand : For Loop Ending Sub

My intern and I wrote a block of VBA that circles through a data set and cleans out rows we do not want before analysis is applied. The loop worked on one day and then the next day it would do the strangest thing: it would appear to execute, but then end the sub and not execute any code after it. We found a solution, which I believe is just better coding in general, that worked, but I don't understand why the original code didn't work and why it was ending the sub. It didn't spit out any errors, it just pretended like it was done and no amount of breaks would stop it mid execution.

Here is the broken block-

LastRow = wb_Audit.Sheets("Data Sheet").Cells(Rows.Count, "AN").End(xlUp).Row
For iRow = 2 To LastRow
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AQ").Value = "Dead Deal" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
            If iRow > LastRow Then End
            Else: End If
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AQ").Value = "Closed" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
            If iRow > LastRow Then End
            Else: End If
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AM").Value = "" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
            If iRow > LastRow Then End
            Else: End If
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AM").Value = "HOLD" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
            If iRow > LastRow Then End
            Else: End If
    Next iRow

Here is the block that we replaced this with that works fine now.

 For iRow = 2 To LastRow
        If iRow > LastRow Then Exit For
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AQ").Value = "Dead Deal" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
        Else: End If
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AQ").Value = "Closed" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
        Else: End If
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AM").Value = "" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
        Else: End If
        If wb_Audit.Sheets("Data Sheet").Cells(iRow, "AM").Value = "HOLD" Then
           wb_Audit.Worksheets("Data Sheet").Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
            iRow = iRow - 1
            LastRow = LastRow - 1
        Else: End If
    Next iRow

We have working code, I just want to understand what was wrong with the first block.

Upvotes: 0

Views: 73

Answers (3)

GMalc
GMalc

Reputation: 2628

Your issue was not deleting from the bottom up, and trying to work around the issues you were encountering. What happens when you delete from top to bottom is; when you test a row and then delete the data, the row below shifts into the current row, your loop then moves to the next row down, causing you to skip the row that shifted up. Like JvdV shows you can combine all your criteria into one If statement. Your code can be shortened a lot...

Dim ws As Worksheet, lastRow As Long

Set ws = ThisWorkbook.Sheets("Data Sheet") 'Or Workbooks("wb_Audit").Sheets("Data Sheet")

lastRow = ws.Cells(ws.Rows.Count, "AN").End(xlUp).Row

For iRow = lastRow To 2 Step -1  'when deleting rows you need to start from the bottom
    If ws.Cells(iRow, "AQ").Value = "Dead Deal" Or ws.Cells(iRow, "AQ").Value = "Closed" Or _
    ws.Cells(iRow, "AM").Value = "" Or ws.Cells(iRow, "AM").Value = "HOLD" Then
    ws.Range("AM" & iRow & ":AW" & iRow).Delete Shift:=xlShiftUp
Next iRow

Upvotes: 0

JvdV
JvdV

Reputation: 75840

Just to show you an alternative to your code:

Sub NewSub()

Dim lr As Long, irow As Long
With ThisWorkbook.Sheets("Data Sheet")
    lr = .Cells(Rows.Count, "AN").End(xlUp).Row
    For irow = lr To 2 Step -1
        If .Cells(irow, "AQ").Value = "Dead Deal" Or .Cells(irow, "AQ").Value = "Closed" Or .Cells(irow, "AM").Value = "" Or .Cells(irow, "AM").Value = "HOLD" Then
            .Range("AM" & irow & ":AW" & irow).Delete Shift:=xlShiftUp
        End If
    Next irow
End With

End Sub

Upvotes: 3

barrowc
barrowc

Reputation: 10679

From the help page for the End statement:

The End statement stops code execution abruptly, without invoking the Unload, QueryUnload, or Terminate event, or any other Visual Basic code

If iRow > LastRow Then End will terminate the macro immediately if iRow > LastRow when we reach this line. Any code after the For iRow = 2 To LastRow loop won't ever be executed because the macro will have terminated.

Rather than using End, use Exit For if all you want to do is to break out of the For loop

Upvotes: 0

Related Questions