1

I'm looking to create a simple procedure which will search for the 'In Shout?' column, search for all '#N/A's' in this column and delete these rows.

When I run the below, it doesn't delete the rows and I can't figure out why. Can anyone see why?

Sub DeleteBadRows()

    Dim InShout As Long
    Dim NA As String
    
    NA = "#N/A"

    'Declaring year value of 1 month & 2 month
    'This is important to compare datasets from 2 months ago & last month
    Year_2M = Format(Date - 57, "YYYY")

    'Declaring month value of 1 month & 2 month
    'This is important to compare datasets from 2 months ago & last month
    Month_2M = Format(Date - 57, "MM")

    'This translates the current month from number to character format
    MonthChar_2 = MonthName(Month_2M, False)

    sheet = "MASTERFILE_" & Year_2M & Month_2M
    
    'setting string values so that we can identify open workbooks
    myFile = "Dataset"
    otherFile = "Monthly Reporting Tool"
    shoutFile = "Copy of Daily Shout"
    
        'if tool wb is open, declare it as MonthlyRepTool
    For Each wb In Application.Workbooks
        If wb.Name Like otherFile & "*" Then
           Set MonthlyRepTool = Workbooks(wb.Name)
        End If
    Next wb
    
        With MonthlyRepTool.Worksheets(sheet).Rows(1)
            Set e = .Find("In Shout?", LookIn:=xlValues)
            InShout = e.Column
        End With

    lastRow = MonthlyRepTool.Worksheets(sheet).Cells(Rows.count, "A").End(xlUp).Row

    For i = 2 To lastRow Step -1

        If MonthlyRepTool.Worksheets(sheet).Cells(i, InShout).value = NA Then
            MonthlyRepTool.Worksheets(sheet).Rows(i).EntireRow.Delete
        End If
        
    Next i

End Sub
ZygD
  • 22,092
  • 39
  • 79
  • 102
  • Your "#N/A" is in cells where you have formulas? Or are they just pasted values? – ZygD Mar 23 '21 at 15:22
  • They're pasted as values. – Danny O'Sullivan Mar 23 '21 at 15:23
  • `#N/A` is actually a type of value, not text. Similar to if you typed `true` into a cell, it would not equal a cell that you typed `="True"` – pgSystemTester Mar 23 '21 at 15:30
  • Are the "#N/A" cells formatted as text, or "General"? That will make a difference. – Tim Williams Mar 23 '21 at 15:33
  • No Need for Looping ;) Simply use Autofilter to filter on `#N/A` and delete the rows. [THIS](https://stackoverflow.com/questions/11317172/delete-row-based-on-partial-text/11317372#11317372) will get you started. The best part about using autofilter is, you do not have to worry about whether the `#N/A` is from formulas or pasted as text. Nor will it matter if the cell is formatted as text or general :) – Siddharth Rout Mar 23 '21 at 16:31

3 Answers3

2
For i = 2 To lastRow Step -1

should be

For lastRow  to 2 Step -1
Tim Williams
  • 154,628
  • 8
  • 97
  • 125
1

You're using a test for the literal TEXT "N/A" when you should be testing for a value of error. If you modify the code below, I think this would probably resolve your issue. Also, (as Tim Williams pointed out), you're going wrong direction. His answer shows one fix. The below changes from going to bottom down. It also is more efficient to delete the rows at the end of the procedure. I included code to make that happen.

Dim KillRange As Range

'change 
For i = 2 To lastRow

If IsError(MonthlyRepTool.Worksheets(Sheet).Cells(i, InShout).Value) Then
    If KillRange Is Nothing Then
        Set KillRange = MonthlyRepTool.Worksheets(Sheet).Rows(i).EntireRow
    Else
        Set KillRange = Union(KillRange, MonthlyRepTool.Worksheets(Sheet).Rows(i).EntireRow)
    
    End If
    
End If

Next i

'Delete after the loop
If Not KillRange Is Nothing Then
    KillRange.Delete
End If
pgSystemTester
  • 8,979
  • 2
  • 23
  • 49
  • 3
    Instead of `IsError()` you can use `Application.IsNa()` which is more specific. – Tim Williams Mar 23 '21 at 15:40
  • I thought of specifying something like that, but figured OP would want to delete any row with an error in that cell. You're right though, your approach would be more surgically specific to the question. – pgSystemTester Mar 23 '21 at 15:53
1

Tested your code. Two things:

  1. Instead of If MonthlyRepTool.Worksheets(sheet).Cells(i, InShout).value = NA Then
    use If Application.IsNa(MonthlyRepTool.Worksheets(Sheet).Cells(i, InShout).Value) Then

  2. Instead of For i = 2 To lastRow Step -1
    use For i = lastRow To 2 Step -1

ZygD
  • 22,092
  • 39
  • 79
  • 102
  • For 1, there's no mention of N/A - is this intentional? Also, I do use Option Explicit. Each of the unmentioned variables has been publicly declared in a different module as I use them across most of my project. – Danny O'Sullivan Mar 23 '21 at 15:42
  • Sorry, I will delete the 3rd statement then. I have edited the code to account for NA errors specifically. Previously I used `IsError` which captures all kinds of errors. – ZygD Mar 23 '21 at 15:43
  • 1
    This worked. Much appreciated ZygD. – Danny O'Sullivan Mar 23 '21 at 15:46