0

I need to filer two different columns and each time delete the visible rows. However, there may not be any rows to delete and as such I've included errHandlers. In the current workbook, there are no rows to be deleted either time. This first instance works fine but the second on throws the runtime error 1004.

Below is a portion of my code which contains bother errHandlers:

Range("T1").Select
With Selection.Interior
    .Pattern = xlSolid
    .PatternColorIndex = xlAutomatic
    .Color = 10498160
    .TintAndShade = 0
    .PatternTintAndShade = 0
End With
   With Selection.Font
    .Color = -16711681
    .TintAndShade = 0
End With
With Selection
    .HorizontalAlignment = xlGeneral
    .VerticalAlignment = xlBottom
    .WrapText = True
    .Orientation = 0
    .AddIndent = False
    .ShrinkToFit = False
    .ReadingOrder = xlContext
    .MergeCells = False
End With
ActiveCell.FormulaR1C1 = "OK or DELETE"
Rows("1:1").Select
Selection.AutoFilter
Range("T2").Select
ActiveCell.FormulaR1C1 = _
    "=IF(AND(RC[-19]<>R[-1]C[-19],RC[-10]<>R[-1]C[-10]),""OK"",IF(AND(RC[-19]=R[-1]C[-19],RC[-10]<>R[-1]C[-10]),""OK"",IF(AND(RC[-19]=R[-1]C[-19],RC[-10]=R[-1]C[-10],RC[-7]=""T""),""OK"",""DELETE"")))"
Selection.AutoFill Destination:=Range("T2:T" & Cells(Rows.Count, 1).End(xlUp).Row)
Range("T2:T" & Cells(Rows.Count, 1).End(xlUp).Row).Select
Selection.Copy
Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
    :=False, Transpose:=False
Application.CutCopyMode = False
ActiveSheet.Range("$A$1:$T" & Cells(Rows.Count, 1).End(xlUp).Row).AutoFilter Field:=20, Criteria1:="DELETE"

On Error GoTo errHandler:

Selection.SpecialCells(xlCellTypeVisible).Select
Selection.EntireRow.Delete

errHandler:
    ActiveSheet.Range("$A$1:$T" & Cells(Rows.Count, 1).End(xlUp).Row).AutoFilter Field:=20
    Columns("E:G").Select
    Selection.ColumnWidth = 11
    Range("G2").Select
    ActiveCell.FormulaR1C1 = _
        "=IF(RC[6]=""T"",""DELETE"",IF(AND(RC[-6]=R[1]C[-6],RC[3]=R[1]C[3],R[1]C[6]=""T""),R[1]C[-1],IF(RC[-6]=R[1]C[-6],R[1]C[-1]-1,VALUE(""06/30/2017""))))"
    Selection.AutoFill Destination:=Range("G2:G" & Cells(Rows.Count, 1).End(xlUp).Row)
    Range("G2:G" & Cells(Rows.Count, 1).End(xlUp).Row).Select
    Selection.Copy
    Selection.PasteSpecial Paste:=xlPasteValues, Operation:=xlNone, SkipBlanks _
        :=False, Transpose:=False
    Application.CutCopyMode = False
      ActiveSheet.Range("$A$1:$T" & Cells(Rows.Count, 1).End(xlUp).Row).AutoFilter Field:=7, Criteria1:="DELETE"

    On Error GoTo errHandler2:

    Selection.SpecialCells(xlCellTypeVisible).Select
    Selection.EntireRow.Delete

errHandler2:
  ActiveSheet.Range("$A$1:$T" & Cells(Rows.Count, 1).End(xlUp).Row).AutoFilter Field:=7
  Range("D2").Select
    ActiveWorkbook.Save
End Sub

Any help would be greatly appreciated.

Tim Williams
  • 154,628
  • 8
  • 97
  • 125
JBowen
  • 3
  • 4
  • 1
    You cannot trigger another error handler from with an error-handling block: see "Error Handling Blocks And On Error Goto" here: http://www.cpearson.com/excel/ErrorHandling.htm Please also see https://stackoverflow.com/questions/10714251/how-to-avoid-using-select-in-excel-vba – Tim Williams Sep 09 '19 at 19:00

1 Answers1

0

You've entered Spaghetti Lane, and you want to get out of there before you dig yourself any deeper.

Your procedure is doing too many things. Doing too many things is the single reason for ever needing more than a single error-handling subroutine in a procedure scope. Break. Things. Down.

An On Error statement has no effect whatsoever if it's executed while VBA is already in an error state - thus, no error-handling subroutine should ever have any On Error statements.

Start by pulling that "delete all visible rows" code into its own procedure scope:

Private Sub DeleteVisibleRows(ByVal source As Range)
    On Error Resume Next
    source.SpecialCells(xlCellTypeVisible).EntireRow.Delete
    On Error GoTo 0
End Sub

Note how Selection is irrelevant in that code, and how no Range needs to be Selected.

Second, an error-handling subroutine should only ever run while VBA is in an error state; not doing that causes the "happy execution path" and the "error execution path" to get intertwined, and that never ends well.

Starting at On Error GoTo errHandler:, your code should look like this:

DeleteVisibleRows Selection 'TODO: work out what Range object this is, use it instead

Dim entireTable As Range
Set entireTable = ActiveSheet.Range("$A$1:$T" & ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row)

entireTable.AutoFilter Field:=20
ActiveSheet.Columns("E:G").ColumnWidth = 11

Dim columnG As Range 'TODO: use meaningful name
Set columnG = ActiveSheet.Range("G2:G" & ActiveSheet.Cells(Rows.Count, 1).End(xlUp).Row)

'NOTE: no need to AutoFill if we write the whole column at once..
columnG.FormulaR1C1 = "=IF(RC[6]=""T"",""DELETE"",IF(...))"
'NOTE: no clipboard gets involved
columnG.Value = columnG.Value ' overwrites formulas with their values

entireTable.AutoFilter Field:=7, Criteria1:="DELETE"
DeleteVisibleRows entireTable

ActiveSheet.Range("D2").Select ' <~ this is the only legit use of Range.Select!!
ActiveWorkbook.Save
Mathieu Guindon
  • 69,817
  • 8
  • 107
  • 235
  • I've not done much VBA coding and as such have used the Excel marco recorder to create a most of this code. I have tweaked it some through searching the web. Yes, the procedure is doing a lot and you've only seen the last part of the code. In your Private Sub DeleteVisibleRows does this retain the header row? It doesn't look like it would. I see where the code you've provided streamlines the procedure but I am having difficulty figuring out how to transition to it. – JBowen Sep 10 '19 at 16:10
  • @JBowen *In your Private Sub DeleteVisibleRows does this retain the header row? It doesn't look like it would.* Yes, yes it does. And the `End Sub` token too. It's a small, separate, specialized procedure that any other code can invoke, and that is good - you can paste it after the `End Sub` of your current procedure, and then that snippet basically replaces everything between `On Error GoTo errHandler:` and `ActiveWorkbook.Save`, near the bottom of your own code. – Mathieu Guindon Sep 10 '19 at 16:15
  • I assume the code would be something like Call DeleteVisisble Rows and I could use this more than once in a procedure? The code I shared started with filtering columnT and deleting any visible rows, then adding the formula to column, filtering column and deleting any visible rows there. My problem in this particular workbook, is that no rows in either column matched the criteria for being a visible row. – JBowen Sep 10 '19 at 16:35
  • Through some trial and error using your suggestion I have managed to get my code to work by using the following after each instance where I want to delete the visible rows after filtering the data: Run DeleteVisibileRows If ActiveSheet.FilterMode Then ActiveSheet.ShowAllData End If I will try to incorporate more of your suggestions to further streamline my code. Thank you for your help! – JBowen Sep 10 '19 at 17:18
  • I tried the midified code in a different workbook and it did not delete the rows that were visible rows as a result of filtering columnT (first instance). Below is the code as I have it from the point of filtering: ActiveSheet.Range("$A$1:$T" & Cells(Rows.Count, 1).End(xlUp).Row).AutoFilter Field:=20, Criteria1:="DELETE" Run DeleteVisibileRows If ActiveSheet.FilterMode Then ActiveSheet.ShowAllData End If In this workbook there were 5 rows to be deleted and nothing happended. – JBowen Sep 10 '19 at 17:55
  • @TimWilliams I cleaned up a lot of the Select statements and removed all error handling – JBowen Sep 11 '19 at 18:44
  • @MathieuGuidon After incorporating your suggestions and further research I got the code to work as follows: With ActiveSheet.UsedRange .AutoFilter Field:=20, Criteria1:="DELETE" .Offset(1).EntireRow.Delete End With ActiveSheet.ShowAllData Thanks for your suggestions for improvement! – JBowen Sep 11 '19 at 18:45