Deleting a row in a For loop messes the objects

Question

I have a table tbl which is a ListObject. I want to delete rows that only have empty values or equations.

Dim tbl As ListObject
Set tbl = sh.ListObjects(1)
Dim r As ListRow
Dim c As Range
Dim d As Integer

For Each r In tbl.ListRows
    d = 1
    For Each c In r.Range.Cells
        If IsEmpty(c) = False And c.HasFormula = False Then
            d = 0
        End If
    Next
        
    If d = 1 Then
        Debug.Print "DELETE", r.Index
        '''' rows(r.Index).EntireRow.Delete
    End If
Next

This works until I uncomment the commented line which deletes the row.

Probably some objects get messed up upon deletion, because error says:

Application defined or object defined error.

Why not use `ListRow.Delete`? Also you need to `Exit For` in the inner loop once you've deleted. Or better, use `Union` to create a range to delete, and only delete *after* looping. — BigBen, Aug 10 '22 at 13:34
@BigBen I made some fixes to the pasted code. Now it is as it should have been. I don't know why I did not use `ListRow.Delete` but I haven't seen this solution yet online. — 71GA, Aug 10 '22 at 13:37
I tried using `r.Delete` instead of the commented line and it ends in the same error. It looks like Microsoft hasn't worked out deletion of the rows well? I removed the extra `Next` - it was my copy / paste mistake. — 71GA, Aug 10 '22 at 13:42
While different in a way, [this is the best solution to delete rows](https://stackoverflow.com/a/72382669/3221380). You just need to set the range within your object, but the priniciple is the same — Sgdva, Aug 10 '22 at 13:51
This is a common problem with collections. You have to delete from the end to the beginning otherwise you are changing the range over which you still need to enumerate. SO for each is out. Its for Range.cells.count to 1 step -1 etc — freeflow, Aug 10 '22 at 15:36

Storax · Accepted Answer · 2022-08-10T13:50:46.847

1

As suggested in one of the comments one could collect the rows to be deleted

Sub loDel()
    Dim tbl As ListObject
    Set tbl = sh.ListObjects(1)
    Dim r As ListRow
    Dim c As Range
    Dim d As Integer

    Dim dRg As Range

    For Each r In tbl.ListRows
    
        d = 1
        For Each c In r.Range.Cells
            If IsEmpty(c) = False And c.HasFormula = False Then
                d = 0
            End If
        Next
        
        If d = 1 Then
            Debug.Print "DELETE", r.Index
            If dRg Is Nothing Then
                Set dRg = r.Range    
            Else
                Set dRg = Union(dRg, r.Range)
            End If
            'Rows(r.Index + 1).EntireRow.Delete
        End If
    Next
    
    If Not dRg Is Nothing Then
       dRg.Rows.Delete
    End If 
End Sub

edited Aug 10 '22 at 13:50

answered Aug 10 '22 at 13:47

Storax

11,158
3
16
33

1

The key was to build up an union of the rows and to delete it at the end. – 71GA Aug 10 '22 at 13:54
1

Yes, right, but it's also possible to use `ListRows.Delete` . You have to step backwards thorugh the rows. – Storax Aug 10 '22 at 14:02
1

@Sortax Wow nice solution! I never knew that this kind of stepping is possible! – 71GA Aug 10 '22 at 14:06
@Storax Depending on the size of your worksheet, deleting rows all it once in an array, versus going one line at a time (even in reverse) could mean your macro takes minutes vs a few seconds. Always best to do impacts to the worksheet all at once to minimize the screen updating, calculations, etc. – pgSystemTester Aug 10 '22 at 15:05
@pgSystemTester Yes, sure, have a look [here](https://stackoverflow.com/questions/47089741/how-to-speed-up-vba-code/47092175#47092175) – Storax Aug 10 '22 at 15:12
@Storax that will of course help, but still inefficient and will inevitably be a slower process. – pgSystemTester Aug 10 '22 at 15:15
1

@pgSystemTester I am aware of that. – Storax Aug 10 '22 at 15:16
@Storax, sorry I mean to tag the OP (71ga), not you. Since you posted both methods, you (likely) already understand the benefits. – pgSystemTester Aug 10 '22 at 15:16
@pgSystemTester No problem. Just a remark: The speed of the code depends on the size of table. Often it does not really matter. But, of course, if we are talking about thousands of rows then it will make a real difference – Storax Aug 10 '22 at 15:18
OMG your post with the standard "speed things up" method got you 14 up votes, and from 2017??? Arghh, I never find the low-hanging fruit. Look how [hard I had to work](https://stackoverflow.com/questions/46509572/excel-formula-based-function-for-sha256-sha512-hashing-without-vba-or-macros/56767828#56767828) to get a 12 score! :-( – pgSystemTester Aug 10 '22 at 15:20
1

Yeah, life can be a b... :-) – Storax Aug 10 '22 at 15:21

score 1 · Answer 2 · answered Aug 10 '22 at 13:59

The other solution would be, as always, to step backwards through the rows

Sub loDel()
    Dim tbl As ListObject
    Set tbl = sh.ListObjects(1)
    Dim d As Integer
    Dim i As Long
    Dim c As Range
    For i = tbl.ListRows.Count To 1 Step -1
        d = 1
    
        Dim rg As Range
        Set rg = tbl.ListRows(i).Range
        For Each c In rg
            If IsEmpty(c) = False And c.HasFormula = False Then
                d = 0
            End If
        Next
        
        If d = 1 Then
            Debug.Print "DELETE", i
            tbl.ListRows(i).Delete
        End If
    Next

End Sub

score 0 · Answer 3 · answered Aug 10 '22 at 18:11

Solution without nested loop:

Option Explicit

Sub test1()
    Dim tbl As ListObject, i As Long, x As Range
    Set tbl = sh.ListObjects(1)
    
    For i = tbl.ListRows.Count To 1 Step -1
        Set x = Nothing
        On Error Resume Next
        Set x = tbl.ListRows(i).Range.SpecialCells(xlCellTypeConstants) 'not empty and not formulas
        On Error GoTo 0
        If x Is Nothing Then
            Debug.Print "DELETED " & i
            tbl.ListRows(i).Delete
        End If
    Next
End Sub

I assume this is much faster... – 71GA Aug 11 '22 at 12:59 — 71GA, Aug 11 '22 at 12:59

score 0 · Answer 4 · answered Aug 11 '22 at 14:58

0

See my recommendation here that is more modular and deletes very fast https://stackoverflow.com/a/73322827/1279373

answered Aug 11 '22 at 14:58

spioter

1,829
1
13
19

Deleting a row in a For loop messes the objects

4 Answers4