1

I've searched online and on here but can't find anything which seems to fit the bill and work. I have got some code which works but because the length of the range changes as it deletes rows it doesn't catch all of the rows to delete and so I end up running it several times which isn't great...so I'm now trying the AutoFilter approach as recommended on this thread.

I have a spreadsheet with several columns, one of which is 'cost'. I need to go through the 'cost' column (which can sometimes be column 9, sometimes 10, hence the 'find' bit below) and delete any row where the cost is 0...

Below is the code I've written so far, any help would be very, very much appreciated!!!!

-- edit 3: some more code changes have been made...new code is below.

Sub RemoveRows()
    Dim cell As Range
    Dim lastRow As Long
    Dim lastColumn As Integer
    Dim criteraColumn As Integer
    Dim criteriaRange As Range

    lastRow = Cells.Find(What:="*", After:=Range("A1"), LookIn:=xlValues, LookAt:=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlPrevious).row
    lastColumn = Cells.Find(What:="*", After:=Range("A1"), LookIn:=xlValues, LookAt:=xlPart, SearchOrder:=xlByColumns, SearchDirection:=xlPrevious).Column

    Rows(1).EntireRow.Select
    criteriaColumn = Selection.Find(What:="Cost", After:=ActiveCell, LookIn:= _
                        xlValues, LookAt:=xlPart, SearchOrder:=xlByRows, SearchDirection:= _
                        xlNext, MatchCase:=False, SearchFormat:=False).Column
    Set criteriaRange = Range("A1", Cells(lastRow, lastColumn))

    criteriaRange.AutoFilter Field:=criteriaColumn, Criteria1:="£0.00"
    AutoFilter.Range.Delete
    ActiveSheet.AutoFilterMode = False
End Sub

When I run this, it applies the filter on the correct range and to the correct column. I then get the error message: "Run-time error '424': Object required" which applies to the AutoFilter.Range.Delete line. When I press 'end' or 'debug' on this and look at the spreadsheet, the filter has been applied to the correct column and the correct option (£0.00) is checked, but no results are returned (I know I need to do some error handling in case of this situation in future but for now there should be at least 10 lines returned by this filter). If I manually press 'OK' on the filter settings in the spreadsheet (without changing any of them!) my 10 results show up correctly so I'm not sure why they're not showing up when I do this programmatically?

Happy to supply spreadsheet example if required, this is really confusing me!!

Spreadsheet can be found here

Elatesummer
  • 303
  • 7
  • 16
  • As has already been mentioned, looping through a range is the least efficient method. Storing the range in an array, then looping through there will provide some speed increase. Another faster way is using the `Find` or `MATCH` functions rather than looping. However, I've found the fastest way is using the Autofilter. There are many other examples (though an answer is provided here) on SO: [delete special rows](http://stackoverflow.com/questions/11317172/delete-row-based-on-condition) [delete row based on condition](http://stackoverflow.com/questions/11317172/delete-row-based-on-condition) etc – Zairja Jul 19 '12 at 14:05
  • Thank you Zairja - This is the method I will be using as suggested by a couple of other people as well, I really appreciate the help from everyone! – Elatesummer Jul 19 '12 at 14:55

4 Answers4

5

You should be able to use the Excel AutoFilter to delete the rows.

Cells.AutoFilter Field:=criteriaColumn, Criteria1:="£0.00"
ActiveSheet.AutoFilter.Range.Offset(1).Delete
ActiveSheet.ShowAllData

This will filter for cost = 0 and then delete all the rows that match the filter.

The operater is automatically equals so you don't need "=£0.00" as the parameter.

I added in the ActiveSheet for AutoFilter to get rid of the object not found, and the Offset makes it not delete headers.

Jon Kelly
  • 316
  • 1
  • 4
  • +1 Great solution and easy to implement. Was about to update my post with it as well, but you beat me to it! – Scott Holtzman Jul 19 '12 at 14:09
  • Awesome - hadn't even thought of that. Thank you very much, Jon - and Scott! – Elatesummer Jul 19 '12 at 14:48
  • I think I must be doing this wrong - I have altered my code (I have edited my original post) but the autofilter doesn't seem to do anything at all? – Elatesummer Jul 19 '12 at 15:30
  • I don't have excel 2007 available to test it on (only 2010), but it seems to be working for me. Make sure that your criteriaColumn is correct. – Jon Kelly Jul 19 '12 at 15:57
  • I edited my answer. It should fix the problem with the currency format. – Jon Kelly Jul 19 '12 at 16:45
  • Thanks - I have also tried this format but although the filter correctly applies, and when I leave the autofilter on and look at the filter settings it has the £0.00 value checked and nothing else, there are no results returned. When I manually go to the filter and press ok (without changing any of the filter settings) it shows the 10 or so results which should be returned. I have 2010 as well so will test it there and then update - I really appreciate all the assistance with this, thank you so much – Elatesummer Jul 20 '12 at 08:35
  • I've updated my code now and explained the issue I'm getting - thank you again – Elatesummer Jul 20 '12 at 09:07
  • I'm not sure why it wouldn't apply the filter, but the change to the delete line should fix the object not found. – Jon Kelly Jul 20 '12 at 12:34
  • Thanks for this, John. Unfortunately I still have the object not found because although the filter is applying, and ticking the '£0.00' value, unless I manually press 'OK' on that filter it doesn't apply. I will put up some links to demonstrate where I've got to on this. – Elatesummer Jul 27 '12 at 10:05
  • @Elate In a couple of your if statements you have `AutoFilter.Range.Rows.Count` change this to `ActiveSheet.AutoFilter.Range.Rows.Count` – Jon Kelly Jul 27 '12 at 13:15
  • Thank you - I've done this now but I'm still getting the same problem as is seen in the test data I posted up...although I seem to get a correct count of 11 rows, if I try the delete function the only one it sees as visible is the header row...I really can't see what I've done wrong here, from all the help everyone here has given me the code now seems to be correct but when I uncomment the delete lines (with ActiveSheet in front of the autofilter!) I still get no rows found to delete...think I may just have to tell the business they have to live without :( – Elatesummer Jul 30 '12 at 08:47
  • If you don't do ActiveSheet.showAllData the auto filter will not be removed. Doing AutoFilterMode = False will just turn off the arrows. – Jon Kelly Jul 30 '12 at 12:15
  • I have added this...unfortunately it still isn't picking up the rows for deletion in the first place so still no further on with this. Does the code work for anyone else when running it? (not that I'm at the edge of desperation here!) – Elatesummer Aug 01 '12 at 08:20
  • Okay the actual problem with this isn't the main code, it's the autofilter itself not applying (even if I record then run a macro it won't apply properly) so I'm going to mark this as answered. If necessary I will raise a specific question around the autofilter separately. Thank you all so very much for your help - you have been invaluable! – Elatesummer Aug 06 '12 at 15:07
2

I would do it in another way, for performance reasons (reading + deleting line by line is slow). I answered a similar question before, I think it is also quite applicable for your post.

What I would suggest that you could do:

  • Instead of reading the sheet row by row, load the entire range into an array once;
  • Instead of deleting row by row, transport the values you need into a second array in memory;
  • When you have the data you need, apply a oRange.clearcontents to clear the current range;
  • Display the new array of data into the sheet by applying oRange = vArray.

Reading / transforming data are performed in memory, and read and write are only performed once.

In this thread, you have the code I wrote before: Execution of VBA code gets slow after many iterations

Let me know if this could help you out, or if you wish to receive more concrete info / code.

Community
  • 1
  • 1
html_programmer
  • 18,126
  • 18
  • 85
  • 158
1

What you need to do is to step backward through the range, since deleting rows automatically moves rows up one by default in Excel.

Replace:

   For Each cell In criteriaRange.Cells
        If cell.Value = 0 Then
            cell.EntireRow.Delete
        End If
    Next cell

With:

Dim i As Integer

For i = criteriaRange.Rows.Count To 1 Step -1
    If criteriaRange.Cells(i) = 0 Then
        criteriaRange.Cells(i).EntireRow.Delete
    End If
Next

Also, ScreenUpdating has no effect on the methods of a macro. It basically turns off "screen-flickering" for every action the macro takes, making it run faster and smoother for the user experience.

Scott Holtzman
  • 27,099
  • 5
  • 37
  • 72
  • Yeah this makes a lot of sense. Going backwards averts any line skipping. – jrad Jul 19 '12 at 13:53
  • This is exactly what I needed and makes perfect sense but unfortunately has completely killed excel when I try to run it - I think it's caught up in the loop so will probably reprogram the way that Kim Gyson has suggested – Elatesummer Jul 19 '12 at 14:05
  • 1
    What you say makes sense, depending on the depth of your data. However, with all due respect to @KimGyson's approach, because it is a great, efficient, approach, you may find that Jon's `AutoFilter` answer is the best in terms of ease of implementation and understanding. AutoFilter is fast and efficient. The only thing I see with his code, is that it may need to be tweaked a bit to fit your data structure. I was actually about to suggest this too, but he beat me to it! – Scott Holtzman Jul 19 '12 at 14:08
0

You could have it so whenever it deletes a row, it either goes back to the top of the column or you could subtract one from the count. Either way, it wouldn't skip any rows. I'd provide actual code but I'm new with VBA and wouldn't quite know how to do that.

jrad
  • 3,172
  • 3
  • 22
  • 24
  • Thanks - I was talking this over with someone and just came to the same conclusion but as I'm using the For loop I think I'd have to get it to exit the loop and then start it again I suppose which doesn't seem very efficient - thank you for your help on this! I'm going to keep searching for code and if I find the answer will post it here - unless someone on here beats me to it! – Elatesummer Jul 19 '12 at 13:46
  • You could do a Do While loop instead of a For loop in that case. Whenever you don't know how many iterations you'll end up doing, do a Do While loop. – jrad Jul 19 '12 at 13:48