I don't get it (vba only works for half the columns)

Question

I. Do. Not. Understand.

I am trying to delete rows based on a value (target located in column 11). This code only works for half the cells i want to delete, so i have to run it like 10 times (it goes from 520, then to 260, then 130, 65...)

My code seems simple enough but i still cannot quite figure out where the issue is...

Thanks for your help

Dim Target As String, i As Integer
Target = "Réduction ligne"
lastrow = Cells(Rows.Count, 11).End(xlUp).Row
For i = 1 To lastrow
    On Error Resume Next
    If Cells(i, 11).Value = Target Then
    Rows(i).EntireRow.Delete = True
   End If
Next i

You should iterate backwards. Otherwise the reference is lost... A better/faster way will be to place all the cells from the rows to be deleted in a (Union) range and then delete its EntireRow at once. — FaneDuru, Jul 02 '21 at 07:34
Also you do not need a loop to delete rows containing "Réduction ligne". Use [AUTOFILTER](https://stackoverflow.com/questions/11317172/delete-row-based-on-partial-text/11317372#11317372). It is much faster. — Siddharth Rout, Jul 02 '21 at 07:36
Has been answered very often at SO, e.g. at [...Delete entire row](https://stackoverflow.com/questions/11767277/excel-vba-if-cell-is-an-integer-delete-the-entire-row/11767564#11767564) — T.M., Jul 02 '21 at 07:36
Wow thanks for the quick reply, i'm new to vba so i still have trouble optimizing my macros — Maximilien Ollier, Jul 02 '21 at 07:40

score 0 · Answer 1 · answered Jul 02 '21 at 07:36

The most important thing first:

On Error Resume Next

Please don't do that. It will make debugging your code a nightmare.

Why does your problem happen:

Now with this out of the way, let's look at the source of your problem. You have the following rows:

1 A
2 B
3 C
4 D

In your first loop iteration i = 1. Row 1 is deleted. Now your data looks like this:

1 B
2 C
3 D

In your second loop iteration i = 2. Since row 1 has been deleted, you are actually looking at the third original row.

How do I fix this:

This problem can be solved by iterating backwards:

For i = lastrow To 1 Step -1

It makes so much sense once you understand how the function works, thanks for the tip — Maximilien Ollier, Jul 02 '21 at 07:41

score 0 · Answer 2 · answered Jul 02 '21 at 07:38

Please, try the next code:

Sub efficientDeleteRows()
  Dim Target As String, lastRow As Long, i As Long, rngDel As Range
 Target = "Réduction ligne"
 lastRow = cells(rows.count, 11).End(xlUp).row
 For i = 1 To lastRow
    If cells(i, 11).value = Target Then
        If rngDel Is Nothing Then
            Set rngDel = cells(i, 11)
        Else
            Set rngDel = Union(rngDel, cells(i, 11))
        End If
    End If
 Next i
 If Not rngDel Is Nothing Then rngDel.EntireRow.Delete
End Sub

I will give that a shot, thanks – Maximilien Ollier Jul 02 '21 at 07:43 — Maximilien Ollier, Jul 02 '21 at 07:43

I don't get it (vba only works for half the columns)

2 Answers2