VBA rows not being deleted correctly

[Dimitris254]

I have a worksheet with 10 columns, two of which (J and K).
J column takes various text values, one of which is "External Research".
K column takes year values (they are formatted as "General" type), starting 2001.

I want to delete the whole row, when J is "External Research" or K is <=2015 (so keep only the current year 2016 and later).

i tried this, but the macro takes just too long (total rows ~50k) :

Option Explicit

Sub clear_years_inputtype()

Dim lastRow As Long
Dim i As Long
Dim wsRAWDATA As Worksheet

Application.ScreenUpdating = False

Set wsRAWDATA = ThisWorkbook.Worksheets("RAWDATA")

With wsRAWDATA
    lastRow = .Cells(.Rows.Count, "A").End(xlUp).Row
    For i = 2 To lastRow
        If .Cells(i, "J") = "External Research" Or .Cells(i, "K") <= 2015 Then
            .Rows(i).EntireRow.Delete
            lastRow = lastRow - 1
            i = i - 1
        End If
    Next i
End With

Application.ScreenUpdating = True


End Sub

What is a better way to do this?

 

[WaterGypsy]

HI

I think the best way to do it is to sort the data so you can delete in bulk.
So first sort on column J and find the start and end of "External Research" then delete those rows in one statement
Then sort by column K, again find the start and end row you want to delete and then delete in one statement.

 

[Momentman]

Instead of using a LOOP macro, why not a FILTER macro,
- filter out those that DONT meet the criteria
- Delete all the visible rows of data that appears
- Remove Filter and you will be left with those that meet your original criteria

 

[Dimitris254]

i wouldn't like to sort the data (it has to remain in the order it is now)

i was thinking maybe a "find & delete" approach is faster though...

 

[Jonmo1]

When deleting rows (or columns), you have to go backwards from the bottom Up (or Right to Left for columns)

Change this
For i = 2 To lastRow

to
For i = lastRow To 2 Step -1

 

[Jonmo1]

Follow through the logic to see why..

Say the criteria in your IF is True for row 10 for example.
Row 10 is then deleted
Whatever data WAS in row 11 is now in row 10.
And Whatever WAS in row 12 is now in Row 11.
But your loop then proceeds to test row 11 (which now contains data originally in row 12)
The original data from row 11 got skipped.


Going backwards avoids this problem.

 

[Momentman]

When deleting rows (or columns), you have to go backwards from the bottom Up (or Right to Left for columns)

Change this
For i = 2 To lastRow

to
For i = lastRow To 2 Step -1

True that @Jonmo, that answers the first question of why his delete macro seems " to be doing the wrong thing"..

But considering the number of rows, i think a Loop will be much slower, which is why I didn't even bother addressing why the Macro appeared not to be working correctly

 

[Dimitris254]

@Momentman: never used the Filter macro, how would that work?

usually i avoid using filtering + delete, so as not to delete the hidden lines

 

[cbaker1991]

Hi,

I think the below does what momentman was thinking of. Test it in a separate file to be sure it does what you expect.

Dim lastRow As Long
Dim wsRAWDATA As Worksheet
Set wsRAWDATA = ThisWorkbook.Worksheets("RAWDATA")
With wsRAWDATA
lastRow = .Cells(.Rows.Count, "A").End(xlUp).Row
Range("$A$1:$K$" & lastRow).AutoFilter Field:=10, Criteria1:= _
"External Research"
Range("$A$1:$K$" & lastRow).Offset(1, 0).SpecialCells _
(xlCellTypeVisible).EntireRow.Delete
End With
wsRAWDATA.AutoFilterMode = False


This code filters on J and removes those which have "External Research" it should be simply to add another section for column K based on the code for J. I have assumed the data starts in column A and the last column is column K in this macro.

 

[Dimitris254]

OMG that was unreal, so fast...

one question while i'm working on removing the redundant years; can i put multiple criteria like Criteria = 2010, 2011, 2012 ?