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?
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.
.Cells(i,1) in that case instead of .Cells(i).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.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
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.A1:A40, and apply the AutoFilter to A1:A20, do you get filtered rows all the way to row 20 or row 40?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.
For Each, you just cannotDeleteinside it without expecting odd behavior (e.g. skipped rows). See theUnion-Deleteexample below.