slow (and wrong?) For...Next loop

  • Thread starter Thread starter Tim
  • Start date Start date
T

Tim

Hi,

Its been a while since i've used VBA so i apologise if i'm doing something
stupid. [XP Pro Excel2002 SP3]

i have a sheet of data. one column may have many entries of 'Ongoing'.
where this occurs, i want to delete the entire row. i've tried the
following: -
'=============
Sub rem_ongoing()
Dim cell As Range
Columns("AI").Select

For Each cell In Selection
If cell.Value = "Ongoing" Then
cell.EntireRow.Delete
End If
Next cell
End Sub
'=============

it works, BUT does not delete all relevant rows (maybe because i'm working
top-down and its missing the row after the one its just deleted?) and is
pretty slow (approx 3000 rows of data, 64 of which contain 'Ongoing' takes
approx 30-40 seconds to complete).

can someone confirm my theory about missing rows and can anyone recommend a
speedier alternative?

thanks

Tim
 
Sub deleterows()
Dim rng As Range
Dim i As Long

Set rng = ActiveSheet.Range(Cells(1, "AI"), Cells(Rows.Count, "AI").End(xlUp))
'Work backwards from bottom to top when deleting rows
Application.ScreenUpdating = False
With rng
For i = .Rows.Count To 1 Step -1
If .Cells(i) = "Ongoing" Then
.Cells(i).EntireRow.Delete
End If
Next i
End With
Application.ScreenUpdating = True
End Sub
 
Hi Tim,

Your suspicions are correct.
This should do you; it creates an array of the rows and deletes them
all in one go - which should be much quicker.

Sub Remove_Unwanted_Rows()
'Highlight range of cells on wksheet;this deletes
'all rows with cells exactly matching cases
Dim Cell, RowArray As Range
For Each Cell In Selection
Select Case Cell
Case Is = "Ongoing" '& any others you like separted by commas
If RowArray Is Nothing Then
Set RowArray = Cell.EntireRow
Else
Set RowArray = Union(RowArray, Cell.EntireRow)
End If
End Select
Next Cell
RowArray.Delete
End Sub

Cheers,
JF
 
Thanks Mike and Josh for the quick replies.

FYI: I can confirm (in my case at least) Josh's method is the quickest (and
way quicker than my attempt).

Cheers

Tim

Hi Tim,

Your suspicions are correct.
This should do you; it creates an array of the rows and deletes them
all in one go - which should be much quicker.

Sub Remove_Unwanted_Rows()
'Highlight range of cells on wksheet;this deletes
'all rows with cells exactly matching cases
Dim Cell, RowArray As Range
For Each Cell In Selection
Select Case Cell
Case Is = "Ongoing" '& any others you like separted by commas
If RowArray Is Nothing Then
Set RowArray = Cell.EntireRow
Else
Set RowArray = Union(RowArray, Cell.EntireRow)
End If
End Select
Next Cell
RowArray.Delete
End Sub

Cheers,
JF
 
Back
Top