Why is this loop not working?

M

Mike R.

Hi -
this seems like a pretty easy loop statement. What am I doing wrong? I am
trying to see if a value in a row equal's the word START and if so, delete
that row.
Help..

irows = ActiveSheet.UsedRange.Rows.Count
For iloop = 2 To irows
If Cells(iloop, "D").Value = "START" Then Cells(iloop,
"D").EntireRow.Delete
Next iloop
 
B

Barb Reinhardt

Two problems:

1) When you delete rows, the counter functionally skips a row
2) if the activerange doesn't include the first row, the rowcount will be off.

Try this

Sub test()
Dim myRange As Range
Set myRange = Nothing
Set myRange = ActiveSheet.Cells(ActiveSheet.Rows.Count, "D")
irows = myRange.End(xlUp).Row
Set myRange = Nothing

For iloop = 2 To irows
If Cells(iloop, "D").Value = "START" Then
If myRange Is Nothing Then
Set myRange = Rows(iloop)
Else
Set myRange = Union(myRange, Rows(iloop))
End If
End If
Next iloop

myRange.EntireRow.Delete
End Sub

HTH,
Barb Reinhardt
 
J

Jim Thomlinson

Not to pick but if the word Start is not found then the code will crash. You
should validate myRange before deleting

if not myRange is nothing then myRange.EntireRow.Delete
instead of just
myRange.EntireRow.Delete
 
G

George Nicholson

Reverse the loop.

For iloop = irows to 2 Step -1

When deleting rows (or any object in a Collection) by index position, start
with the LAST member, and work forward.
The reason is that once you delete, the "next" highest element fills the
current index position...but you just evaluated the current position and are
about to move off of it. Result: every time you delete you end up 'skipping'
the next highest row/object and it is never evaluated. This isn't an issue
if you work backwards from the end of the collection.

When its possible to use a For Each..Next structure, this isn't an issue.
Each member will be evaluated.

HTH,
 
J

Jennifer

For testing purposes I made a test sub that I ran manually. Worked
for me. How are you running it?
 
B

Barb Reinhardt

You're right. I missed that.

Jim Thomlinson said:
Not to pick but if the word Start is not found then the code will crash. You
should validate myRange before deleting

if not myRange is nothing then myRange.EntireRow.Delete
instead of just
myRange.EntireRow.Delete
 
D

David Hilberg

- As soon as you delete a row, another moves up to fill its place,
possibly with a START value that now won't be examined.
- If your first row is blank, UsedRange will have 1 fewer row than you
thought.
- You lost your Endif

Assuming your first row isn't blank:

irows = ActiveSheet.UsedRange.Rows.Count
For iloop = irows To 2 Step -1
If Cells(iloop, "D").Value = "START" Then
Cells(iloop, "D").EntireRow.Delete
End If
Next iloop
 
T

Tim879

When you're deleting a row, you mess up the index in excel.

Think of it this way... you're up to row 3.... your index (iloop in
your case) =3. You decide to delete row 3. Excel deletes row 3 and
then moves all of the rows in the spreadsheet up 1 row. So now, what
was row 4 became row 3.

Check out this site.... it will likely have some code you can use.
http://www.cpearson.com/excel/deleting.htm
 
P

par_60056

Hi -
this seems like a pretty easy loop statement. What am I doing wrong? I am
trying to see if a value in a row equal's the word START and if so, delete
that row.
Help..

irows = ActiveSheet.UsedRange.Rows.Count
For iloop = 2 To irows
If Cells(iloop, "D").Value = "START" Then Cells(iloop,
"D").EntireRow.Delete
Next iloop

A couple common problems:
1) When you delete a row this way you will miss the next row
because the row counter increments.
2) Loop looks to be an integer which could be a problem if you have
a lot of rows.

Try:

Dim lLoop As Long

For lLoop = ActiveSheet.UsedRange.Rows.Count +
ActiveSheet.UsedRange.Row To 2 Step -1
If (UCase(Cells(lLoop, "D")) = "START") Then Rows(lLoop).Delete
shift:=xlUp
Next

Peter Richardson
 
Z

Zone

Mike, you really should go from the bottom when deleting rows:
For iloop=irows to 2 step -1
James
 
T

Tim879

your index is messed up.

consider this example. ILoop = 3, excel is now reviewing row 3 and it
turns out that this row should be deleted. Excel deletes row 3 and
moves all of the other rows up 1 (i.e. row 4 becomes row 3). Your
ILoop increments by 1, so now... ILoop = 4 but the current row should
be reviewing in excel is row 3.

check out:
http://www.cpearson.com/excel/deleting.htm

you should be able to modify that code to do what you need.
 
I

INTP56

Alternate method:

Option Explicit

Public Sub DeleteRows(WS As Worksheet, strSearch As String, intColumnNumber
As Integer)
Dim rngHitCell As Range
Set rngHitCell = WS.Columns(intColumnNumber).Find _
(What:=strSearch _
, LookIn:=xlFormulas _
, LookAt:=xlWhole _
, SearchOrder:=xlByRows _
, SearchDirection:=xlNext _
, MatchCase:=False _
, SearchFormat:=False _
)

While Not rngHitCell Is Nothing
WS.Rows(rngHitCell.Row).Delete
Set rngHitCell = WS.Columns(intColumnNumber).FindNext
Wend
End Sub

Public Sub TestDeleterows()
DeleteRows WS:=ThisWorkbook.Worksheets(1), strSearch:="START",
intColumnNumber:=4
End Sub

Bob
 
I

INTP56

Alternate method 2 -

You could also implement this by passing a search range in instead of a
worksheet and column number. This solves the problem of deleting START in row
1.

With the first method, you could grab the value of .Cells(1,4), change it,
run the PROC, then change it back. Or you could trap when rngHitCell was on
row 1, etc.

BTW, is there a nice way of specifying Columns(4) except for row 1?
Something like Exclude(rng1,rng2) that returns rng1 minus what's in rng2.

Bob

Option Explicit

Public Sub DeleteRows2(rngSearch As Range, strSearch As String)
Dim rngHitCell As Range
Set rngHitCell = rngSearch.Find _
(What:=strSearch _
, LookIn:=xlFormulas _
, LookAt:=xlWhole _
, SearchOrder:=xlByRows _
, SearchDirection:=xlNext _
, MatchCase:=False _
, SearchFormat:=False _
)

While Not rngHitCell Is Nothing
rngSearch.Parent.Rows(rngHitCell.Row).Delete
Set rngHitCell = rngSearch.FindNext
Wend
End Sub

Public Sub TestDeleteRows2()
With ThisWorkbook.Worksheets(1)
DeleteRows2 rngSearch:=.Range(.Cells(2, 4), .Cells(65536, 4)),
strSearch:="START"
End With
End Sub
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top