For loop in Excel VBA only deleting every 2nd row

Question

I thought I'd solved this problem with the help of answers here but I'm still only deleting every second row that needs deleting. The loop is:

For Each row In ActiveSheet.UsedRange.Rows
    'test for v in cell f and delete if there isn't one
    str = Cells(row.row, "F").Value
    Debug.Print "str is " & str
    If InStr(1, str, "V") <> 0 Then
        Debug.Print "hello str is " & str
    Else
        row.Delete Shift:=xlUp
    End If
Next row

But when I run it on the following rows:

I end up with:

M1301   PMH
M1401   Rod Washer
M1502   Rod Washer
MV1502  Rod Washer
MV1503  Rod Washer

I feel like I'm going crazy. I had an incrementing counter in the loop as well and thought that was the problem but I still have it even though I've stopped using the counter to reference rows.

Any help in pointing out what I assume is obvious would be much appreciated.

Thanks

Tyr to give feedback and/or credit this time to people who assisted you below. You have not done in previous posts you asked here and got good answers. Do you know how to mark an answer as "ANSWER" ? — Shai Rado
– Shai Rado, Commented Aug 31, 2017 at 5:53

Dan Donoghue · Accepted Answer · 2017-08-31 05:42:16Z

3

Because you delete a row row.Delete Shift:=xlUp for example row 4, row 5 now becomes row 4 when you deleted the row and you then go to the next row (row 5 which is the old row 6).

You can either put Row = Row - 1 after your delete OR you could do it a different way which is to go backwards.

Example

For X = range("A" & rows.count).end(xlup).row to 2 step - 1
    'Do something per row
    'Delete a row if need be
Next

This should give you enough of an idea to fix this.

answered Aug 31, 2017 at 5:42

Dan Donoghue

6,2662 gold badges20 silver badges36 bronze badges

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

3 Comments

Shai Rado Over a year ago

Nice detailed explanation

Madeline Over a year ago

Thanks. What's the purpose of xlup in brackets after end?

AntiDrondert Over a year ago

@Madeline It stands for up arrow.

Shai Rado · Accepted Answer · 2017-08-31 05:46:33Z

2

Try the code below, I tried to use as much of your original logic (even though there are easier and shorter ways to do it).

Explanation inside the code as comments,

Note: in General, allways loop backwards when deleting Objects, or Rows in your case.

Code

Option Explicit

Sub DeleteV()

Dim Rng As Range, i As Long, LastRow As Long
Dim Str As String

' I would rather use Worksheets("SheetName") instead
Set Rng = ActiveSheet.UsedRange

LastRow = Rng.Rows.Count + Rng.Row - 1 ' just in case your range starts from the 2nd row (or 3rd...)

' allways loop backwards when deleting rows
For i = LastRow To Rng.Row Step -1
    'test for v in cell f and delete if there isn't one
    Str = Cells(i, "F").Value
    Debug.Print "str is " & Str

    If InStr(1, Str, "V") <> 0 Then
        Debug.Print "hello str is " & Str
    Else
        Rows(i).Delete
    End If
Next i


End Sub

answered Aug 31, 2017 at 5:46

Shai Rado

33.7k7 gold badges32 silver badges55 bronze badges

1 Comment

Madeline Over a year ago

Thanks. I suspected there's probably better ways of doing it and now the example going backwards make a lot more sense to me. Can I ask why you'd use Worksheets("SheetName") rather than ActiveSheet?

Nicholas Bucher Ampudia · Accepted Answer · 2017-08-31 05:49:57Z

1

The moment you erase a row, all following row indexes decrease by one.

This means, if you erase row n because it contains a "V", row n+1 is now row n and will not be tested.

Hence this, your code skips every second row.

To fix it, try going through the rows backwards.

answered Aug 31, 2017 at 5:49

Nicholas Bucher Ampudia

418 bronze badges

Comments

Michał Turczyn · Accepted Answer · 2017-08-31 05:59:33Z

1

I can suggest some changes in code, that will help:

'always store reference to the sheet in a variable!
Dim sh As Worksheet
Set sh = ActiveSheet
'determine last row in F column
lastRow = sh.Cells(sh.Rows.Count, 6).End(xlUp).Row

For i = lastRow To 1 Step -1
    'test for v in cell f and delete if there isn't one
    'we make it uppercase, to avoid situation that we didn't match v with V
    str = UCase(sh.Cells(i, 6).Value)
    Debug.Print "str is " & str
    If InStr(1, str, "V") > 0 Then
        Debug.Print "hello str is " & str
    Else
        Rows(i).Delete
    End If
Next i

edited Aug 31, 2017 at 5:59

answered Aug 31, 2017 at 5:44

Michał Turczyn

41.2k18 gold badges58 silver badges87 bronze badges

3 Comments

Shai Rado Over a year ago

you want to loop forward when deleting rows ?

Madeline Over a year ago

Thanks - what's the reasoning for always storing the reference to the sheet in a variable?

Michał Turczyn Over a year ago

Because, when using ActiveSheet you are using the sheet which is currently active, which might be different from the sheet you are running macro in. Thus, it might give unexpected results.

Collectives™ on Stack Overflow

For loop in Excel VBA only deleting every 2nd row

4 Answers 4

3 Comments

1 Comment

Comments

3 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

3 Comments

1 Comment

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related