Loop for Deleting Rows

Hello Experts

Whats wrong with my code ?

for i=3 to tblrows2
  if range ("a" & i) = 1 then
  rows ("i:i").select
  selection.delete shift:=xlup
  end if
next i

Thanks
Sharky

When you delete rows you should work in reverse - bottom up...

If you consider you have rows 2:10 and you loop from 2 to 10, when you delete row 3 then what was row 4 is now 3 yet you miss it as you go to 4 (which was 5), make sense ?

Better then to adopt something along the lines of:

For i = tblrows2 to 3 step -1
    If Cells(i,"A") = 1 then Rows(i).Delete
Next i

Exactly right...and if you're deleting a LOT of rows, then the code will execute much faster if you turn off the screenupdating:

Application.ScreenUpdating = False
    For i = tblrows2 to 3 step -1
        If Cells(i,"A") = 1 then Rows(i).Delete
    Next i
Application.ScreenUpdating = True

If speed were a concern then I would make the point that as much as ScreenUpdating will slow code, given Deletion of Rows is a Volatile action if you have Volatile functions you're best served forcing Calculation to Manual prior to implementation of Deletion Loop and restoring Calc to it's former setting on completion ... and if you do that you may as well do all three, ie: screenupdate, calculation & events.

An example below:

Public Sub Example()
Dim xlCalc As XlCalculation, tblRows2 As Long, i As Long
On Error GoTo Handler
tblRows2 = Cells(Rows.Count, "A").End(xlUp).Row
With Application
    xlCalc = .Calculation
    .Calculation = xlCalculationManual
    .ScreenUpdating = False
    .EnableEvents = False
    For i = tblRows2 To 3 Step -1
        If Cells(i, "A") = 1 Then Rows(i).Delete
    Next i
ExitPoint:
    .Calculation = xlCalc
    .ScreenUpdating = True
    .EnableEvents = True
End With
Exit Sub

Handler:
MsgBox "Error " & Err.Number & " (" & Err.Description & ")"
Resume ExitPoint

End Sub