On Error Resume Next

Despite everything that’s been written about it, sometimes On Error Resume Next is the perfect tool for the job. Say you have a ListObject on Sheet1, that would be named Table1; you could have a Table1 property that would return this ListObject:

'@Description("Gets the 'Table1' ListObject from this sheet.")
Public Property Get Table1() As ListObject
  Set Table1 = Me.ListObjects("Table1")
End Property

The only problem, is that when the Table1 table is inevitably renamed (right?) in Excel, this property starts raising run-time error 9, because the ListObjects collection of Sheet1 doesn’t contain an item named "Table1" anymore, and throwing [an error] from a Property Get procedure goes against property design best practices. Enter On Error Resume Next:

'@Description("Gets the 'Table1' ListObject from this sheet, or 'Nothing' if not found.")
Public Property Get Table1() As ListObject
  On Error Resume Next ' suppresses possible error 9...
    Set Table1 = Me.ListObjects("Table1") ' ...that would be raised while evaluating RHS
  On Error GoTo 0 ' execution would jump here before the Table1 reference is assigned
End Property

Error handling is promptly explicitly restored with On Error GoTo 0, and the property will now return Nothing and defer to the caller the decision of what to do with an invalid table:

Dim table As ListObject
Set table = Sheet1.Table1
If table Is Nothing Then Exit Sub ' or raise an error? MsgBox?
'...safely carry on...

A nice side-effect of this, is that it’s very compelling for the calling code to capture the returned value into a local variable – and leaving the complexities of caching concerns to the calling code (you don’t want/need to dereference that ListObject from the worksheet’s ListObjects collection every single time you need to access it!) makes it much less likely to run into an awkward situation such as this:

'@Description("Gets the 'Table1' ListObject from this sheet.")
Public Property Get Table1() As ListObject
  Static cache As ListObject
  If cache Is Nothing Then
    Set cache = Me.ListObjects("Table1")
  End If
  Set Table1 = cache
End Property

...
  Debug.Print Me.Table1.Name
  ...
  Me.ListObjects(1).Delete
  ...
  Debug.Print Me.Table1.Name ' error 424 "object required"
...

Assuming the table initially exists on the worksheet, the first call to Me.Table1 sets the cache reference and the calling instruction outputs the table’s name. In the second call to Me.Table1, the cache reference is already set, it’s not Nothing anymore – the object pointer is zombified: there’s a pointer, but the object itself is gone, and the corrupted cache state might very well be persisted until an End instruction kills the entire execution context. And that’s why cache invalidation is up there together with naming things at the top-2 of hard things in programming… but I digress.

On Error Resume Next + Conditionals = Trouble

Of all the things that could go wrong with generously suppressing error handling by spraying On Error Resume Next all over the code base, is that potentially catastrophic bugs like below can happen – what’s the MsgBox saying, and what icon does it have? Is that clearly intentional?

Public Sub AntiExample()
  'On Error GoTo Rubberduck
  On Error Resume Next
  ' ...code...
  Sheet1.Range("A1").Value = CVErr(xlErrNA) ' A1 contains a #N/A error value
  ' ...code...
  ' ...
  ' ...more code...
  ' ...
  If Sheet1.Range("A1").Value = 42 Then
    MsgBox Err.Description, vbCritical
    Exit Sub
  Else
    MsgBox Err.Description, vbExclamation
    Exit Sub
  End If
  Exit Sub
Rubberduck:
  MsgBox Err.Description, vbInformation
End Sub

Did you guess it right? I’m only going to tell you that Resume Next can be extremely dangerous if it’s used wrong: a MsgBox is harmless, but that conditional block could contain anything. When in doubt, On Error GoTo Rubberduck, but if you choose to use Resume Next anyway, there’s an inspection that can warn you when it is used without being paired with On Error GoTo 0 (in most common scenarios anyway) – but it’s not that inspection’s job to tell you there’s too much code between On Error Resume Next and On Error GoTo 0: that is entirely up to you… but the way I see it, OERN is great for fencing a single potentially problematic statement – and that is easier to do when the procedure is responsible for very few things: when you start having multiple potential errors to handle in the same scope, it’s past time to think about increasing the abstraction level and moving code to smaller procedures that do fewer things.

Pragmatically speaking, if used correctly, On Error Resume Next does not really need to be paired with On Error GoTo 0: execution state is always local to the procedure (/stack frame), and with On Error Resume Next the execution state will never be in error after the procedure exits. In a sense, specifying it explicitly is a bit like specifying an explicit Public access modifier, or an explicit ByRef specifier on a parameter declaration: it’s the implicit default, made explicit – on the other hand, it’s much cleaner to exit a procedure with Err.Number being 0, consistent with the execution/error state.

When you see On Error Resume Next at the top of a rather large procedure, comment it out and run the code if possible; see what errors are being silently shoved under the carpet, what code executes in that uncertain error state. Try to narrow down the potential errors to specific statements, and isolate them: reduce the amount of code that is allowed to run in an error state to a bare minimum, pull the “dangerous” statements into their own function, see if there’s a way to avoid needing to handle an error in the first place. In the above code for example, the error raised here:

If Sheet1.Range("A1").Value = 42 Then

Could easily be avoided by verifying whether we’re looking at a value that can legally be compared to 42 (or any other non-Error value), like this:

Dim cellValue As Variant
cellValue = Sheet1.Range("A1").Value ' cellValue is Variant/Error

If IsNumeric(cellValue) Then ' false
  If cellValue = 42 Then ' comparison is safe in this branch
     '...
  End If
ElseIf Not IsError(cellValue) Then ' false
  'cellValue isn't an error value, but isn't numeric either
  '...
Else
  'execution branches here
  '...
End If

Of course that’s just one example… and every situation is different: if you’re reading a Recordset and one of the fields is missing, you have all rights to blow things up… but you still have to make sure you clean up the mess and close the connection properly before you exit the scope – but then consider, if you were given the opened connection as a parameter… life is much simpler: it’s not your job at all to close that connection – whoever created it will be assuming it’s still open when the failing procedure returns! The basic golden rule of thumb being that the code that’s responsible for creating an object (or invoking a factory method that creates it) should also be the code that’s responsible for destroying that object.