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.