1

I am trying to create a Macro that will find each cell that contains a specific value (e.g.:Location) then deletes the whole row of that cell. There will be multiple cells that contain that value and I want all of them deleted so I have created a loop but because of the way I've done it, it breaks after the last one and it doesn't carry on with the last bit of the macro. I cannot find another way around so I am hoping someone will be smarter than me and can help me out with this one. The code looks like this now:

  Do
   If Cells.Find(What:="Location").Activate Then
    ActiveCell.EntireRow.Select
    Selection.Delete
   Else MsgBox ("All headers deleted")
   End If
  Loop
Community
  • 1
  • 1
K. Robert
  • 101
  • 2
  • 12

3 Answers3

3

You're actually pretty close. Here's how to make it work, with some minor "not necessary but still good to have" improvements:

Sub test()
    Dim rng As Range
    Do
        Set rng = Cells.Find(What:="b")
        If Not rng Is Nothing Then
            rng.EntireRow.Delete
        Else
            MsgBox ("All headers deleted")
            Exit Do
        End If
    Loop
End Sub

The reason I added a Range object and declare it before we even enter the If loop is mostly for readability but also so we don't attempt to use .Find several times. It's unlikely this has any major impact - the compiler probably fixes this by itself. Regardless, it doesn't hurt to make it explicit in the code.

The reason you get an error after the last instance is deleted is because the code tries to .Activate an empty range. This operation isn't permitted. One solution, namely the one I've gone for here, is to check whether the range object is valid before attempting to call any of its members. If the object isn't valid, the loop will skip it altogether and we don't get any errors. The way this works is, Range.Find returns Nothing (which is a valid object state) if it doesn't find anything. You'll see in my code that I am telling the compiler to enter the loop only if rng doesn't contain Nothing.

I also removed all .Select calls, because it's not good practice. Best practice is to declare all variables explicitly and working directly on ranges. The resulting code is also shorter and easier to read.

The final edit is adding Exit Do in your Else clause. This is highly necessary since you didn't add any break condition to your Do ... Loop. Without an Exit Do somewhere, the program would toss MsgBox at you forever.

It's also possible to move from Exit Do to a more robust Do ... Loop with conditions - it could look like this:

Sub test()
    Dim rng As Range
    Do
        Set rng = Cells.Find(What:="b")
        If Not rng Is Nothing Then
            rng.EntireRow.Delete
        Else
            MsgBox ("All headers deleted")
        End If
    Loop While (Not rng Is Nothing)
End Sub
Vegard
  • 3,587
  • 2
  • 22
  • 40
  • Brilliant thank you! Also very good explanation, I have learned a lot from it! 5*! – K. Robert Nov 17 '17 at 11:06
  • The only problem now is that after the MsgBox is coming up it doesn't carry on with the macro because there is a final bit to it after all that data was deleted. Any idea why? – K. Robert Nov 17 '17 at 11:51
  • You mean there's a cell it didn't delete? – Vegard Nov 17 '17 at 11:57
  • No, after it deleted all of the cells, the message box pops up but there is are more actions after the loop which it won't do for some reason. So let's say that after the loop I put in actions where I copy and paste the data to somewhere else, it will just simply skip it and will end the macro after I press "Ok" on the MsgBox. – K. Robert Nov 17 '17 at 15:38
  • Ok, well, if you used the solution from earlier with `Exit Sub`, then that is the problem. See the updated answer which uses `Exit Do` which will resume the rest of the macro. – Vegard Nov 17 '17 at 15:39
1

Use FindNext. In this example it builds up the found cells to be deleted and set them all to a variable. Then it deletes them all in one go - This is much faster if you have many cells to delete then doing them one by one.

Dim DelRng As Range, fndCell As Range
Dim firstAddress As String

With ActiveSheet
    Set fndCell = .Cells.Find(What:="Location")
    If Not fndCell Is Nothing Then
        firstAddress = fndCell.Address
        Do
            If DelRng Is Nothing Then
                Set DelRng = fndCell
            Else
                Set DelRng = Union(DelRng, fndCell)
            End If
            Set fndCell = .Cells.FindNext(fndCell)
        Loop Until fndCell.Address = firstAddress
    End If
    If Not DelRng Is Nothing Then
        DelRng.EntireRow.Delete
        MsgBox ("All headers deleted")
    Else
        MsgBox ("No headers found")
    End If
End With
Tom
  • 9,725
  • 3
  • 31
  • 48
-1

This worked for me.

Set cell = Selection.Find(What:=valueToFind, After:=ActiveCell, LookIn:=xlFormulas, _
    LookAt:=xlWhole, SearchOrder:=xlByRows, SearchDirection:=xlNext, _
    MatchCase:=False, SearchFormat:=False)
If Not cell Is Nothing Then
    cell.Select
 else
   msgbox "Cell with given Data not found"
end if
Sumit Shitole
  • 110
  • 1
  • 3
  • 20
  • Sticking this in the `Do` loop would cause recursion. You need to handle the exit. – Tom Nov 17 '17 at 10:58