Despite everything that’s been written about it, sometimes
On Error Resume Next is the perfect tool for the job. Say you have a
Sheet1, that would be named
Table1; you could have a
Table1 property that would return this
'@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
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
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.