2
    For Each c In oSheet.Range("A1:A1000")
        If InStr(c.Value, "VALUE") Then
            c.EntireRow.Delete()
        End If
    Next

This will only delete a few of the rows within the specified range, what could the problem be?

3
  • 2
    You will probably next to reserve the loop to go from the last cell for first cell. Otherwise it'll skip rows that get deleted. Commented May 27, 2015 at 16:25
  • You shouldn't use a For ... Each loop in order to remove elements within the collection, as you're modifying the number of elements in that collection within the loop. It would be better to use a For I = 1000 To 1 Step -1 loop as @Gareth mentioned. Commented May 27, 2015 at 17:49
  • @ChicagoMike, you can use the For Each, you just cannot Delete inside it without expecting odd behavior (e.g. skipped rows). See the Union-Delete example below. Commented May 27, 2015 at 19:55

2 Answers 2

3

Here are two common patterns for deleting entire rows based on a condition. The main idea is that you cannot delete from a collection while you iterate it. This means that Delete should not appear in a For Each This is fairly standard across most programming languages and some even throw an error to prevent it.

Option 1, use an integer to track the rows and have it work from the end to the beginning. You need to go backwards because it is the easy way to avoid skipping rows. It is possible to go forwards, you just need to account for not incrementing when you delete.

Sub DeleteRowsWithIntegerLoop()

    Dim rng_delete As Range
    Set rng_delete = Range("A1:A1000")

    Dim int_start As Integer
    int_start = rng_delete.Rows.Count

    Dim i As Integer
    For i = int_start To 1 Step -1
        If InStr(rng_delete.Cells(i), "VALUE") > 0 Then
            rng_delete.Cells(i).EntireRow.Delete
        End If
    Next i

End Sub

Option 2, use the Union-Delete pattern to build a range of cells and then delete them all in one step at the end.

Sub DeleteRowsWithUnionDelete()


    Dim rng_cell As Range
    Dim rng_delete As Range

    For Each rng_cell In Range("A1:A1000")
        If InStr(rng_cell, "VALUE") > 0 Then
            If rng_delete Is Nothing Then
                Set rng_delete = rng_cell
            Else
                Set rng_delete = Union(rng_delete, rng_cell)
            End If
        End If
    Next

    rng_delete.EntireRow.Delete

End Sub

Notes on the code

For Option 2, there is an extra conditional there to create rng_delete when it starts at the first item. Union does not work with a Nothing reference, so we first check that and if so, Set to the first item. All others come through and get Set by the Union line.

Preference

When choosing between the two, I always prefer Option 2 because I much prefer to work with Ranges in Excel instead of iterating through Cells with a counter. There are limitations to this. The second option also works for discontinuous Ranges and all other variety of weird Ranges (e.g. after a call to SpecialCells) which can make it valuable when you are not sure what data you will be dealing with.

Speed

I am not sure about a speed comparison. Both can be slow if ScreenUpdating and calculations are enabled. The first option makes N-1 calls to Delete whereas the second option does a single one. Delete is an expensive operation. Option 2 does however make N-1 calls to Union and Set. I assume it's faster than the first one based on that (and it seems to be here), but I did not profile it.

Final note: InStr returns an integer indicating where the value was found. I always like to make the boolean covnersion explicit here and compare to >0.

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

5 Comments

Great detailed answer!
Very good answer, Thankyou very much! The only problem I get is when running it i get this error: - Additional information: Conversion from type 'Range' to type 'String' is not valid On this line of code: - If InStr(rng_delete.Cells(i), "APO") > 0 Then
Might mean your range is more than a single column. Use .Cells(i,1) in that case instead of .Cells(i).
As the rng_delete Union is more common than a non-union it will speed the code up if you change the order of the IF test.
@brettdj, Flip the If-Else with a Not or put the inner If (...Is Nothing...) on the outside? I don't want to Set rng_delete until I know there is a match so I don't think the latter change is viable.
1

Dont use loops, use a method such as AutoFilter

Sub Recut()
Dim ws As Worksheet
Dim rng1 As Range

Set ws = ActiveSheet
Set rng1 = ws.[A1:A100]

ws.AutoFilterMode = False

Application.ScreenUpdating = False
With rng1
      .AutoFilter Field:=1, Criteria1:="*VALUE*"
      `assuming A1 is a header, and checking for at least 1 match on the filtered string
       If .SpecialCells(xlCellTypeVisible).Cells.Count > 1 Then .Offset(1, 0).Resize(rng1.Rows.Count - 1, 1).EntireRow.Delete
End With
ws.AutoFilterMode = False
Application.ScreenUpdating = True

End Sub

7 Comments

So far as I can tell, AutoFilter ignores the size of the range that it is being applied to. That is, calling [A1:A20].AutoFilter with data in rows 21:40 also will extend the AutoFilter to all 40 rows. This is fine except in the case where the filter returns no results. The deletion will then be done on the original range [A1:A20].EntireRow (really A2:A21) which is entirely hidden so it will all get deleted. Normally the AutoFilter-Delete works because Excel will only delete visible rows when applied to a filtered range. If all go hidden, then it doesn't work and deletes all.
@byron - no the range doesnt extend.
If you put data in A1:A40, and apply the AutoFilter to A1:A20, do you get filtered rows all the way to row 20 or row 40?
Row 20. But you make a good point re deleting all rows if there are no matches - will address
Very odd. With a column of 1s (so that no rows match *VALUE*) from A1:A40, if I run your code with range A1:A20, and stop it after the AutoFilter is applied (don't delete), rows 2:40 are hidden. Excel 2013.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.