Deleting rows based on 2 column citeria

[Rob342]

HI
I need to delete data off a database depending on what value is in column J & Column K
for example if col k is month 9 & column K =2014 then delete the row and shift up leaving no blank lines

The following routine works, but it wont delete the row for some reason ?

Private Sub CommandButton48_Click()
'// New routine to delete data from the Dams MDB Sheet 16/09/2015
'   Display warning to the user in the routins just in case
'   Get the month & Year from the user
'   Month is on Column J - Year is on Column K
Dim ws As Worksheet
Dim LR As Long, i As Long
Set ws = Worksheets("test")
'Application.ScreenUpdating = False
    
With ws
    LR = .Range("A" & Rows.count).End(xlUp).Row 'Last Row
    For i = 40 To 2 Step -1                       'LR To 2 Step -1
        If ws.Range("J" & i).Value = "9" And ws.Range("K" & i).Value = "2014" Then _
        Rows(i).EntireRow.Delete
    Next i
End With
'Application.ScreenUpdating = True
        
End Sub

[p45cal]

Perhaps the values in columns J and K are not text but numbers?Try removing the quote marks from around the numbers.

[jolivanes]

This might be faster then looping.
Try on a copy of your workbook first though

Sub Maybe
    Range("A1", Range("K1048576").End(xlUp)).AutoFilter 10, 9
    Range("A1", Range("M1048576").End(xlUp)).AutoFilter 11, 2014
    Range("A2", Range("A1048576").End(xlUp)).EntireRow.Delete
    ActiveSheet.AutoFilterMode = False
End Sub

BTW, what is actually shown in the columns for the month and year?
Do they say 9 and 2014?

[Rob342]

Hi P45cal
Have removed the quotes and it worked all ok
Any ideas on how to improve the speed as its a bit slow deleting 2000 records
Thanks for you assistance
jolivanes
I try your routine tomorrow & let you know ok
What do the numbers represent ie("K1048576")
Also the values input could change depending on the user input, what month & year they select
Rob

[jolivanes]

Re:Any ideas on how to improve the speed as its a bit slow deleting 2000 records
See post #3

1048576 is the bottom (last) cell in Excel 2007 and higher.
If you have Excel 2003 or earlier, use 65536
Or re-write it as long as it comes out to the last used cell in that column

[Rob342]

Hi jolivanes
Tried the routine
run time error 1004 Autofilter method of range class failed on this line Range("A1", Range("M1048576").End(xlUp)).AutoFilter 11, 2014
Have changed the M to a K
Rob

[p45cal]

I've been comparing speeds amongst the above methods and a couple more, on a sheet I made up with 6k rows with about 300 rows scattered about randomly eligible for deletion.
All methods times are trivial; less then half a second.
The autofilter was the fastest.
Rob342, how long is it taking with your trials? How many rows are we talking?
Perhaps a sample workbook?

[Rob342]

Hi p45cal

This is what I have come up with, for 31000 rows to search & to delete 2500 rows its taking about 3 seconds
Probably not going to get any better than that, using autofilter is the fastest way, can you see any problems with the code ?

lastrow = cells(Rows.count, 1).End(xlUp).Row
With Range("J1:K1").Resize(lastrow)
    .AutoFilter field:=1, Criteria1:=Val(Me.TxtDDRM.Text)
    .AutoFilter field:=2, Criteria1:=Val(Me.TxtDDRYR.Text)
    If Application.WorksheetFunction.Subtotal(2, .Columns(1)) > 1 Then _
        .Resize(.Rows.count - 1).Offset(1).SpecialCells(xlCellTypeVisible).EntireRow.Delete
    .AutoFilterMode = False

Regards Rob

[mancubus]

if i put my two cents in

    With Worksheets("MySheet")
        .AutoFilterMode = False
        .Range("J1").CurrentRegion.AutoFilter Field:=1, Criteria1:=Val(Me.TxtDDRM.Text)
        .Range("J1").CurrentRegion.AutoFilter Field:=2, Criteria1:=Val(Me.TxtDDRYR.Text)
        .AutoFilter.Range.Offset(1).SpecialCells(xlCellTypeVisible).EntireRow.Delete
        .AutoFilterMode = False
    End With

or

    With Worksheets("MySheet").Range("J1").CurrentRegion
        .Parent.AutoFilterMode = False
        .AutoFilter Field:=1, Criteria1:=Val(Me.TxtDDRM.Text)
        .AutoFilter Field:=2, Criteria1:=Val(Me.TxtDDRYR.Text)
        .Parent.AutoFilter.Range.Offset(1).SpecialCells(xlCellTypeVisible).EntireRow.Delete
        .Parent.AutoFilterMode = False
    End With

[p45cal]

careful mancubus.. range("J1").currentregion.autofilter Fields 1 and 2 will be different from Range("J1:K1").Resize(lastrow).autofilter Fields 1 and 2 !

[mancubus]

that's true p45cal.
if it is a poorly designed table and both Jx and Kx (x being any same row number) are blank cells.

[p45cal]

I was actually saying that the table probably extends to column A since the OP's original code used column A to determine LR. So J1.currentregion.autofilter fields 1 and 2 will probably be columns A and B, when the OP is filtering on columns J and K

[mancubus]

it's better to design 'contiguous range' tables whose topleft cells are A1 or Cells(1,1) or Cells(1)

in that case:

 With Worksheets("MySheet")
     .AutoFilterMode = False
     .Cells(1).AutoFilter Field:=9, Criteria1:=Val(Me.TxtDDRM.Text)
     .Cells(1).AutoFilter Field:=10, Criteria1:=Val(Me.TxtDDRYR.Text)
     .AutoFilter.Range.Offset(1).SpecialCells(xlCellTypeVisible).EntireRow.Delete
     .AutoFilterMode = False
 End With

[Rob342]

Mancubus & p45cal
I have tried the latest code all works ok & very fast in region on 1 sec to delete 2500 rows
Have changed the code to read field 10 & 11 on autofilter, reading the discussion I have enclosed a sample of the data for you ok
BTW col J & K always has data in the fields cannot be blank
Rob

[mancubus]

Rob

that's a good table (starts at A1, no blank cells in first row and first column) and you can work with it easily whichever way you like.

with the code i posted, i wanted to Show that you dont need to test if the autofilter returns no rows (Subtotal bit of the code you posted).

autofilter is my favourite to delete rows as well.

[p45cal]

That was a hastily put together file wasn't it?:

No proper sub
Uses fields 9 & 10 instead of 10 & 11
No TextDDRM/TextDDYR controls
All records are 9/2014

[Rob342]

Hi mancubus
Thanks for the comments, always like to keep everything neat & tidy where possible
Will certainly be using this code in future projects

p45cal
It was just too show the layout of the database as all the controls are in a multipage form and its too big to post.


Many thanks for all the help
Rob