VBA rows not being deleted correctly

Dimitris254

Board Regular
Joined
Apr 25, 2016
Messages
139
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) :
Code:
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?
 

Excel Facts

Fastest way to copy a worksheet?
Hold down the Ctrl key while dragging tab for Sheet1 to the right. Excel will make a copy of the worksheet.
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.
 
Upvote 0
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
 
Upvote 0
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...
 
Upvote 0
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
 
Upvote 0
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.
 
Last edited:
Upvote 0
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 :)
 
Upvote 0
@Momentman: never used the Filter macro, how would that work?

usually i avoid using filtering + delete, so as not to delete the hidden lines :(
 
Upvote 0
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.
 
Upvote 0
OMG that was unreal, so fast... :eeek:

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

Forum statistics

Threads
1,223,239
Messages
6,170,947
Members
452,368
Latest member
jayp2104

We've detected that you are using an adblocker.

We have a great community of people providing Excel help here, but the hosting costs are enormous. You can help keep this site running by allowing ads on MrExcel.com.
Allow Ads at MrExcel

Which adblocker are you using?

Disable AdBlock

Follow these easy steps to disable AdBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the icon in the browser’s toolbar.
2)Click on the "Pause on this site" option.
Go back

Disable AdBlock Plus

Follow these easy steps to disable AdBlock Plus

1)Click on the icon in the browser’s toolbar.
2)Click on the toggle to disable it for "mrexcel.com".
Go back

Disable uBlock Origin

Follow these easy steps to disable uBlock Origin

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back

Disable uBlock

Follow these easy steps to disable uBlock

1)Click on the icon in the browser’s toolbar.
2)Click on the "Power" button.
3)Click on the "Refresh" button.
Go back
Back
Top