For loop deleting rows doesn't work, need to run macro multiple times

Question

Working Environment: Excel 2013
Target: Delete the unnecessary columns by filtering the content in row 2.

My idea is that as long as the content in row 2 is either

Physical Location or
PLC Tag Name or
Test Step1/2/3/4/5/6/7,

keep those columns, otherwise delete it.

My problem is that I need to run this macro multiple times to delete all the unnecessary columns. It should loop from 1 to 40, and just leave the columns that I want. I am not sure why it doesn't work. Can anyone help me? Thanks!

My code:

Sub Reorder()
    Rows(1).Insert shift:=xlShiftDown

    For i = 1 To 40
        WY = Worksheets("Sheet4").Cells(2, i)
        Select Case WY
            Case "Physical Location"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "PLC Tag Name"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step1"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step2"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step3"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step4"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step5"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step6"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case "Test Step7"
                Worksheets("Sheet4").Cells(1, i) = 1
            Case Else
                Worksheets("Sheet4").Cells(1, i) = 0
        End Select
    Next i

    For i = 1 To 40
        If Worksheets("Sheet4").Cells(1, i) = 0 Then
            Columns(i).EntireColumn.Delete
        End If
    Next
End Sub

Loop from 40 to 1.

BrakNicku
– BrakNicku

2017-05-12 07:14:23 +00:00
Commented May 12, 2017 at 7:14 — BrakNicku
– BrakNicku, Commented May 12, 2017 at 7:14

Graham · Accepted Answer · 2017-09-21 15:41:59Z

3

If you delete rows in a loop, you need to start from the bottom. Otherwise the row number of the rows below the actual deleted row decrease by one and i gets increased by one (from the loop), so in sum you miss a line.

instead of

For i = 1 To 40

use

For i = 40 To 1 Step -1
    If Worksheets("Sheet4").Cells(1, i) = 0 Then
        Columns(i).EntireColumn.Delete
    End If
Next

to loop backwards.

Side note: (thx to @A.S.H)
You should use full qualified ranges/cells/rows/columns and never assume the worksheet. Also declare all your variables using option explicit.

Therefore

Rows(1).Insert shift:=xlShiftDown
'...
Columns(i).EntireColumn.Delete

should be

Worksheets("Sheet4").Rows(1).Insert shift:=xlShiftDown
'...
Worksheets("Sheet4").Columns(i).EntireColumn.Delete

So in sum we end up at

Option Explicit 'first line in module

Public Sub Reorder()
    Dim i As Long
    Dim WY As Worksheet

    Set WY = Worksheets("Sheet4")

    WY.Rows(1).Insert shift:=xlShiftDown

    For i = 1 To 40
        Select Case WY.Cells(2, i)
            Case "Physical Location", "PLC Tag Name", "Test Step1", "Test Step2", _
                 "Test Step3", "Test Step4", "Test Step5", "Test Step6", "Test Step7"
                WY.Cells(1, i) = 1
            Case Else
                WY.Cells(1, i) = 0
        End Select
    Next i

    For i = 40 To 1 Step -1
        If WY.Cells(1, i) = 0 Then
            WY.Columns(i).EntireColumn.Delete
        End If
    Next
End Sub

Or if we use only one loop which is a lot faster:

Option Explicit 'first line in module

Public Sub Reorder()
    Dim i As Long
    Dim WY As Worksheet

    Set WY = Worksheets("Sheet4")

    WY.Rows(1).Insert shift:=xlShiftDown

    For i = 40 To 1 Step -1
        Select Case WY.Cells(2, i)
            Case "Physical Location", "PLC Tag Name", "Test Step1", "Test Step2", _
                 "Test Step3", "Test Step4", "Test Step5", "Test Step6", "Test Step7"
                WY.Cells(1, i) = 1
            Case Else
                WY.Columns(i).EntireColumn.Delete
        End Select
    Next i
End Sub

edited Sep 21, 2017 at 15:41

Graham

7,86020 gold badges67 silver badges92 bronze badges

answered May 12, 2017 at 7:14

Pᴇʜ

57.9k10 gold badges55 silver badges75 bronze badges

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

4 Comments

A.S.H Over a year ago

Just for completeness; qualify the first line Worksheets("Sheet4").Rows(1).Insert shift:=xlShiftDown ( to avoid another question ;) )

Freddy Wang Over a year ago

@Peh Thanks! I don't even realize that I need to loop backwards. Now I know.

Freddy Wang Over a year ago

@A.S.H Thank you too! Keeping a good practice in the beginning is always good to everyone. thanks again to you two!

Pᴇʜ Over a year ago

Hah and still place for improvements; I just realized that it is totally unnecessary to have 2 loops so we can reduce run time a lot. So maybe you can even omit writing the 1 into column 1 if this was just a helper column.

Collectives™ on Stack Overflow

For loop deleting rows doesn't work, need to run macro multiple times

1 Answer 1

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related