0

I am having an issue. I have written code to find and select the second column in a table immediately below the title 'Association'. However, the next step is to go through the selected cells/range, select all rows with a value of 0.00% and then delete them.

For this I have written the following:


'Setting Variables

    Dim cell As Range
    Dim rngData As Range

'Selecting Range to go through and re-formatting

    Sheets("Allocations").Select
    Columns("A:A").Select
    Selection.Find(What:="Association", After:=ActiveCell, LookIn:=xlFormulas _
        , LookAt:=xlPart, SearchOrder:=xlByRows, SearchDirection:=xlNext, _
        MatchCase:=False, SearchFormat:=False).Activate
    Range("A19").Select
    Range(Selection, Selection.End(xlDown)).Select
    Selection.EntireRow.Hidden = False
    ActiveCell.Offset(0, 1).Range("A1").Select
    Range(Selection, Selection.End(xlDown)).Select
    Selection.Style = "Percent"
    Selection.NumberFormat = "0.0%"
    Selection.NumberFormat = "0.00%"

'Attempting to delete just cells within current selection with a value of Zero

    Set rngData = Selection
    For Each cell In rngData
        If cell.Value = 0 Then
        Rows.Delete
        End If
    Next cell

End Sub

Right now it just deletes everything from the workbook which....isn't ideal haha

Appreciate any help I can get on this, I've done a lot of googling and am now pulling my hair out!

Thanks

1 Answers1

1

It is a good practice to explicitly qualify the object you're working with. That is a fancy way of saying, "identify the object"

You've may have read something like that before and I'm certain you'll read something similar again and again. The problem with your code happens to be a great example of why it's a good practice.

Consider the phrasing, "it is a good practice".

What am I saying, or more relevantly, what am I implying?

I am not explicitly saying that you are required to qualify the object; in fact, I am implicitly saying that you have a choice, that it is not a requirement.

That is exactly what you've done with this line: Rows.Delete

Rows is a property of the worksheet object and properties do not exist in a vacuum, they always accompany the object they belong to. You did not explicitly identify the worksheet object, rather, you implicitly allowed VBA to use the default worksheet object.

So, implicitly, you wrote this: ActiveSheet.Rows.Delete

And that is exactly what your macro does, it deletes the rows of the active sheet.

I recommend running the code after replacing it with this line: cell.EntireRow.Delete

If you do that, you should notice something funny. Whereas before, you were consistently deleting every row on the worksheet, now you're inconsistently deleting as few as 50% of the rows you want to delete.

Why? We fixed it, what happened?

Removing elements from an indexed list is a common problem for new programmers. To use a board game as an analogy. You have the entire game planned out. You make one move, then another, and so on. But you've completely neglected your opponent. They get to take a turn too!

Let me ask you this, if you delete the first row in a worksheet, what row number will the worksheet start with?

So what's happening, is you delete a line and Excel immediately shifts the lines up with row 2 becoming row 1. So when you advance to to row 2, your actually skipping row 2 and landing in 3.

The good news is that it's an easy problem to solve. Just go backwards. In the case of a table, that means starting at the last row and moving to the first row.

You can do this with a For loop using the optional Step parameter with a value of -1.

For example (pun half intended):

For I = objTable.Listrows.Count to 1 Step -1

Unfortunately, you have to use an indexing iterator. For Each does not work Step.

Now your code should work as expected.

But before class is dismissed, let's talk about Select.

In a word: "Don't"

As in "you're not a macro recorder so don't write code like one". It will make your code slow. It still make your code more prone to errors and bugs.

So there you have it. Your homework is to learn how to explicitly qualify objects to avoid implicitly using ActiveSheet. How to use Step in a For loop. And how to avoid using Select

ProfoundlyOblivious
  • 1,455
  • 1
  • 6
  • 12
  • Thank you very much for the detailed explanation! I am just starting my learning of VBA so stuff like this really helps :) With regards to 'Select' can I simply take it out of the code altogether or do I need to replace it with something? Thanks again! – Jonny Stowell Feb 05 '20 at 19:56
  • There is a lot of nuance involved with removing `Select`. There isn't a one size fits all solution. The best thing you can do is read up and practice with the intent to improve. Here's a link to get you started. https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba If you find my answer helpful, and it solved your question, then please consider accepting it as the solution to your post. – ProfoundlyOblivious Feb 05 '20 at 20:04