For loop end variable doesn't change

Question

I've written a very simple loop that goes through a table column and colors negative values red, positive values green and removes empty rows.

The problem occurs when rows are deleted. I update the value of the RowCount, and compensate i to check the same row again since a row was just deleted. If I have a column with 10 rows of which 2 are empty, they are deleted. I would expect the For i = 1 to RowCount to stop at 8, but it continues to 9 and produces an error because it then tries to delete the nonexistent 9th row.

What do I need to do so the loop stops at 8 instead of continuing (to I assume the initial value of the RowCount.

Sub ColourFilledCells()
    Dim Table1 As ListObject
    Set Table1 = ThisWorkbook.Worksheets(1).ListObjects(1)

    Dim i As Lon, RowCount As Long

    RowCount = Table1.ListRows.Count

    For i = 1 To RowCount
        If Not Table1.DataBodyRange(i, 1) = Empty Then
            With Table1.DataBodyRange(i, 1)
                If .Value < 0 Then
                    .Interior.Color = RGB(255, 0, 0)
                ElseIf .Value > 0 Then
                    .Interior.Color = RGB(0, 255, 0)
                Else
                    .ColorIndex = 0
                End If
            End With
        ElseIf Table1.DataBodyRange(i, 1) = Empty Then
            Table1.ListRows(i).Delete
            RowCount = RowCount - 1
            i = i - 1
        End If
    Next i

End Sub

Just loop backwards `For i =RowCount to 1 step -1` and get rid of the two counter in the `ElseIf` — Scott Craner, Dec 21 '15 at 03:50
①Use conditional formatting to achieve your `.Interior.Color` color changes. ② To remove a fill, use `.Interior.Pattern = xlNone` not `.Interior.ColorIndex = 0` ③ see [this](http://stackoverflow.com/questions/34387929/looping-through-rows-in-a-listobject-to-delete-them-is-very-slow/34389018#34389018) to improve the time it takes to delete rows. — , Dec 21 '15 at 06:24
@Jeeped, Thanks, 1, Would be easier but I don't want the colors to change until the very end when a button is pressed. 2, got nothing on that :) 3. I've been toying with Fadi's solution and now I'm just using the loop to create ranges which are colored/ deleted as needed outside the loop. — SilentRevolution, Dec 21 '15 at 21:47

chris neilsen · Accepted Answer · 2015-12-21T21:46:48.573

3

To avoid issues with Delete affecting to For loop, count backwards.

Your code, refactored (Plus a few suggestions)

For i = RowCount to 1 Step -1
    If Not isempty( Table1.DataBodyRange(i, 1)) Then
        With Table1.DataBodyRange(i, 1)
            If .Value < 0 Then
                .Interior.Color = vbRed
            ElseIf .Value > 0 Then
                .Interior.Color = vbGreen
            Else
                .ColorIndex = xlColorIndexNone
            End If
        End With
    Else 
        Table1.ListRows(i).Delete
    End If
Next i

edited Dec 21 '15 at 21:46

answered Dec 21 '15 at 04:12

chris neilsen

52,446
10
84
123

Great, I didn't know you change the step to take to a negative number. I take it the `Else Then` Should be simply `Else` – SilentRevolution Dec 21 '15 at 14:27

score 1 · Answer 2 · answered Dec 21 '15 at 04:23

Try this code :

Sub ColourFilledCells()
Dim Table1 As ListObject
Dim uRng As Range

Set Table1 = ThisWorkbook.Worksheets(1).ListObjects(1)

Dim i As Long, RowCount As Long

RowCount = Table1.ListRows.Count

For i = 1 To RowCount
    If Not Table1.DataBodyRange(i, 1) = Empty Then
        With Table1.DataBodyRange(i, 1)
            If .Value < 0 Then
                .Interior.Color = RGB(255, 0, 0)
            ElseIf .Value > 0 Then
                .Interior.Color = RGB(0, 255, 0)
            Else
                .ColorIndex = 0
            End If
        End With
    ElseIf Table1.DataBodyRange(i, 1) = Empty Then            
        If uRng Is Nothing Then
          Set uRng = Table1.ListRows(i).Range
        Else
          Set uRng = Union(uRng, Table1.ListRows(i).Range)            
        End If

    End If
Next i

If Not uRng Is Nothing Then uRng.Delete xlUp

End Sub

This is great, it's much faster as all rows are deleted at the same time outside the loop. I haven't worked much with ranges so far, what does the `xlUp` in `uRng.Delete xlUp` do? — SilentRevolution, Dec 21 '15 at 14:22
@SilentRevolution, see: [Range.Delete Method](https://msdn.microsoft.com/EN-US/library/office/ff834641.aspx) . `xlUp` and `xlShiftUp` both have the same value: `-4162` — Fadi, Dec 21 '15 at 21:02

For loop end variable doesn't change

2 Answers2

Linked