VBA For loop iteration issue

Question

In the code below, I am trying to check 2 date values. If they exist, calculate the days between in column BG. If they don't exist or result is less than 0 then delete the row.

The issue I am having is that once it deletes, a row, it uses Next I, and skips the row directly after it. Ex: row 1 & 2 are missing a date. row 1 gets deleted. row 2 gets pushed up to row 1. then next i, so we are on row 2 now(which was 3) and skips row 2's results. Using i=i-1 seems to crash my program. Also, is there any further way to make my code efficent so that it can iterate through thosands of items faster?

Sub Func4()

Dim N As Long, i As Long, j As Long, cnt As Long, date1 As Date, date2 As Date, date3 As Long ', iold As Long

N = Cells(Rows.Count, "A").End(xlUp).Row

j = 2
cnt = 0
For i = 2 To N 'main
    j = j + 1
    'iold = i
    If Not IsEmpty(Cells(i, "AB").Value) And Not IsEmpty(Cells(i, "AE").Value) Then
        date1 = Cells(i, "AB").Value 'AB=Entry Date
        date2 = Cells(i, "AE").Value 'AE=Rec'd
        date3 = Work_Days(date2, date1)
        cnt = cnt + 1

        If date3 >= 0 Then
            Cells(i, "BG").Value = date3

        Else
            Rows(i).EntireRow.Delete
            'i = i - 1 'HERE
        End If
    Else
        Rows(i).EntireRow.Delete
        'i = i - 1 'HERE
    End If

    'End If
    'If i = iold Then
Next i

'Else
'Next
'End If

End Sub

RESOLVED WORKING ANSWER:

Sub Func4()
   Dim N As Long, i As Long, j As Long, cnt As Long, date1 As Date, date2 As Date, date3 As Long
    N = Cells(Rows.Count, "A").End(xlUp).Row
    j = 2
    For i = N To 2 Step -1
            j = j + 1

        If Not IsEmpty(Cells(i, "AB").Value) And Not IsEmpty(Cells(i, "AE").Value) Then
            date1 = Cells(i, "AB").Value 'AB=Entry Date
            date2 = Cells(i, "AE").Value 'AE=Rec'd
            date3 = Work_Days(date2, date1)
            cnt = cnt + 1
                If date3 >= 0 Then
                    Cells(i, "BG").Value = date3

                Else
                    Rows(i).EntireRow.Delete
                 End If
        Else
        Rows(i).EntireRow.Delete
        End If
        Next i

End Sub

Thank you for the tip! I never thought about it that way. I have seemed to get it working perfect! Please submit your simple response as an answer and I'll reward you points! — Josh, Sep 14 '16 at 12:47

score 0 · Answer 1 · answered Sep 14 '16 at 12:47

First question was answered by Comintern and Shai Rado in the comments:

Always loop through items in reverse as deleting items from the range will shift the remaining items back. By reverse looping the deleted items won't affect the remaining items.

Second question, to speed things up, read this blog post:

https://blogs.office.com/2009/03/12/excel-vba-performance-coding-best-practices/

and especially the part under: Read/Write Large Blocks of Cells in a Single Operation

It will be a -lot- faster if you process your data in Arrays, read and write back the data into an array as shown in the example in this blog. Especially with thousands of items this will be significant!

score 0 · Accepted Answer · edited May 23 '17 at 12:09

To solve the first problem you could run your loop in reverse. That way a deletion only shifts rows which have already been iterated over.

For i = N To 2 Step -1

The second question is a bit open ended. This guide has a few suggestions on how to make your code faster:

http://datapigtechnologies.com/blog/index.php/ten-things-you-can-do-to-speed-up-your-excel-vba-code/

A few items from the guide that stand out the most to me related to your code.

"Disable Sheet Screen Updating"

Place the following around your code, especially if you anticipate a lot of the rows being deleted.

    Application.ScreenUpdating = False
    Application.ScreenUpdating = True

"Avoid Excessive Trips to the Worksheet"

This would be best accomplished by reading columns "AB" and "AE" into an array, and iterating through it rather than cells on the sheet. Here is a solution to accomplish that:

Fastest way to read a column of numbers into an array

Dim Ar as Variant

Ar = Sheets("Sheet").Range("A1:A10000").Value

score -1 · Answer 3 · answered Sep 14 '16 at 12:37

-1

Change For i = 2 To N to Do until i > N and Next i to Loop (don't forget to assign i = 2 beforehand) and then comment the line below the delete row

answered Sep 14 '16 at 12:37

Jeremy

1,337
3
12
26

The point is that deleting rows in a loop will shift the items causing you to 'miss' some iterations. When deleting items from the looped items list always loop in reverse. – Maarten van Stam Sep 14 '16 at 12:51
I agree that in nearly all circumstances you should loop in reverse but not _always_ - there's certain exceptions – Jeremy Sep 14 '16 at 12:59
Sure there are always exceptions, but best practice -in VBA that reads to always- loop in reverse when deleting items from the collection you are iterating over. Take it from me, this is one of the top mistakes made by people starting to develop against the Office Object Model. So be careful teaching starting Office developers to do otherwise. – Maarten van Stam Sep 14 '16 at 13:06
Point taken @MaartenvanStam – Jeremy Sep 14 '16 at 13:07

VBA For loop iteration issue

3 Answers3