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
– Scott Craner, Commented Dec 21, 2015 at 3: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 to improve the time it takes to delete rows. — user4039065
– user4039065, Commented Dec 21, 2015 at 6: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
– SilentRevolution, Commented Dec 21, 2015 at 21:47

chris neilsen · Accepted Answer · 2015-12-21 21:46:48Z

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, 2015 at 21:46

answered Dec 21, 2015 at 4:12

chris neilsen

53.3k10 gold badges91 silver badges127 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

SilentRevolution Over a year ago

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

Fadi · Accepted Answer · 2015-12-21 04:23:31Z

1

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

answered Dec 21, 2015 at 4:23

Fadi

3,3223 gold badges21 silver badges43 bronze badges

2 Comments

SilentRevolution Over a year ago

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?

Fadi Over a year ago

@SilentRevolution, see: Range.Delete Method . xlUp and xlShiftUp both have the same value: -4162

Collectives™ on Stack Overflow

For loop end variable doesn't change

2 Answers 2

1 Comment

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related