Reputation: 174
I'm creating a macro that calls various functions, all of which return strings, and displays the results in a textbox.
I'm starting to read up on good error handling practices, but having a tough time really understanding a lot of it. I'm hoping for some opinions on how I'm going about this.
Basically, the method that I've implemented so far for error handling is to put an error handler at the beginning of each of my various functions, and set it up so that if an error occurs in that function, it will notify the user but continue on to calculate the other functions.
Each one of my functions would look similar to this
Function fnGetNumbers() As String
On Error Goto ErrorHandler
// Code to extract numbers from text
If NumbersInText = vbNullString Then
fnGetNumbers = vbNullString
Else
fnGetNumbers = "The numbers in the text are: " & NumbersInText
End If
ErrorHandler:
If Error <> 0 Then
fnGetNumbers = "An error occurred while extracting numbers from the text."
End If
End Function
Any thoughts and/or advice would be much appreciated!
Upvotes: 2
Views: 3198
Reputation: 1615
I'll tell you what I usually do. I have a sub routine specifically for exiting the macro (I call it exitPoint), and I have a sub routine controling the flow (I call it main), at the begining of main I have a boolean called badExit set to true, and at the end of main I set it to false, then finally call exitPoint. Each sub routine or function has an error trap which will pass control over to ExitPoint with a string saying which routine the error was in. exitPoint then runs a series of clean up and error handeling code, depending on wether the badExit was true or false.
Bascially the idea for this is I would be providing support, if it was a macro I was handing off to someone never to see it again I would put a lot more defensive coding and helpful error messages in there - you can test for an error number and give a specific message for that for example.
SO something like this (this is an actual macro that i've cut lots of code out of just to illustrate):
Option Explicit
Option Private Module
...
Private mbBadExit As Boolean
Private msMacroWbName As String
Private msMacroWbPath As String
Private miSaveFormat As String
Private miSheetsInNewWb As String
Private mcolWorkbooks As New Collection
Private mwbkNew As Workbook
...
Sub Main()
' ---------------------------------------------------------------------
' Control procedure
' ---------------------------------------------------------------------
Debug.Print "Main Start " & Time
'set exit state
mbBadExit = True
'set macro document name and path
msMacroWbName = ThisWorkbook.Name
msMacroWbPath = ThisWorkbook.Path
miSaveFormat = Application.DefaultSaveFormat
miSheetsInNewWb = Application.SheetsInNewWorkbook
'disable some default application behaviours for macro effeciency
With Application
.Calculation = xlCalculationManual
.ScreenUpdating = False
.DisplayAlerts = False
.EnableEvents = False
.DisplayStatusBar = False
.DefaultSaveFormat = xlOpenXMLWorkbook 'for excel 2007 compatability
.SheetsInNewWorkbook = 3
End With
Debug.Print "AddNew Start " & Time
AddNew 'creates new workbook which the rest of the macro works with
Debug.Print "Import Start " & Time
Import 'import bobj CP_Import file and scalepoint data
Debug.Print "Transform Start " & Time
Transform 'various data munging to final state
mbBadExit = False 'set exit state for clean exit
Debug.Print "ExitPoint Start " & Time
ExitPoint 'single exit point
End Sub
Private Sub ExitPoint(Optional ByVal sError As String)
' ---------------------------------------------------------------------
' Single exit point for macro, handles errors and clean up
' ---------------------------------------------------------------------
Dim mwbk As Workbook
'return application behaviour to normal
On Error GoTo 0
With Application
.DisplayAlerts = True
.Calculation = xlCalculationAutomatic
.ScreenUpdating = True
.EnableEvents = True
.DisplayStatusBar = True
.DefaultSaveFormat = miSaveFormat
.SheetsInNewWorkbook = miSheetsInNewWb
End With
'handle good or bad exit
If mbBadExit = False Then 'no problem
MsgBox "Process complete"
'close this workbook, leaving result workbook open
Application.DisplayAlerts = False
Set mcolWorkbooks = Nothing 'destroy collection object
Workbooks(msMacroWbName).Close 'close macro wbk
Application.DisplayAlerts = True
Else 'an error occured
'show user error details
MsgBox prompt:="Macro process has ended prematurely. Contact ... for support." _
& IIf(sError <> vbNullString, vbCrLf & sError, vbNullString) & vbCrLf _
& Err.Description, Title:="Error " & IIf(Err.Number <> 0, Err.Number, vbNullString)
On Error Resume Next
'clean up open workbooks
For Each mwbk In mcolWorkbooks
mwbk.Close
Next
End If
Debug.Print "Finish " & Time
End
End Sub
Private Sub AddNew()
' ---------------------------------------------------------------------
' Creates new workbook which is the base workbook for
' The rest of the macro
' ---------------------------------------------------------------------
On Error GoTo errTrap
Set mwbkNew = Workbooks.Add
mcolWorkbooks.Add mwbkNew
With mwbkNew
.Title = "CP HR Import"
.Subject = "CP HR Import"
End With
Exit Sub
errTrap:
ExitPoint ("Error in AddNew sub routine") 'pass control to error handling exitpoint sub
End Sub
Private Sub Import()
' ---------------------------------------------------------------------
' Connect to source file (xlsx) with ADO, pull data into a recordset
' with SQL, then pull data to the workbook from the recordset to a
' querytable. Kill connection etc..
' ---------------------------------------------------------------------
On Error GoTo errTrap
...Code here...
Exit Sub
errTrap:
ExitPoint ("Error in Import sub routine") 'pass control to error handling exitpoint sub
End Sub
Sub Transform()
' ---------------------------------------------------------------------
' Looks for records with an increment date and inserts a new record
' showing the new scalepoint from the increment date with the new
' salary
' ---------------------------------------------------------------------
On Error GoTo errTrap
...code here...
Exit Sub
errTrap:
ExitPoint ("Error in Transform sub routine") 'pass control to error handling exitpoint sub
End Sub
Sub ColumnsToText(rngColumns As Range)
' ---------------------------------------------------------------------
' Takes a column as a range and converts to text. UK date safe but
' not robust, use with care.
' ---------------------------------------------------------------------
Dim avDates As Variant
avDates = rngColumns.Value
With rngColumns
.NumberFormat = "@"
.FormulaLocal = avDates
End With
Exit Sub
errTrap:
ExitPoint ("Error in ColumnsToText sub routine") 'pass control to error handling exitpoint sub
End Sub
Upvotes: 0
Reputation: 55672
Suggest that you wouldn't run the ErrorHandler section by default for this, ie either
Err.Raise 999
below)If I was running a long subroutine with cleanup (ie restoring Application
settings then I would have the ErrorHandler above the default clean-up (along with handling the fact the error had occurred).
So perhaps this (couldn't resist slight tweak to the IF as well - fnGetNumbers is null by default so no need to set it)
code
Function fnGetNumbers() As String
On Error GoTo ErrorHandler
`test the error
Err.Raise 999
If NumbersInText <> vbNullString Then fnGetNumbers = "The numbers in the text are: " & NumbersInText
Exit Function
ErrorHandler:
fnGetNumbers = "An error occurred while extracting numbers from the text."
End Function
Upvotes: 2
Reputation: 2392
Error handling (in my opinion) is really going to come down to the kind of macros you are writing, who is using them, and who is debugging them. While proper and thorough error handling is best practice, if you are the only on ever debugging them then you will be the only one ever needing custom errors. This changes depending on your organization, but it comes down to what you need.
That being said, some notes on your code:
Function fnGetNumbers() As String
' Instead of returning a string, you can return a boolean and pass in a
' holder string for returning the value. This allows you to check TRUE/FALSE
' instead of checking if a string holds an error.
On Error Goto ErrorHandler
// Code to extract numbers from text
If NumbersInText = vbNullString Then
fnGetNumbers = vbNullString
Else
fnGetNumbers = "The numbers in the text are: " & NumbersInText
End If
Exit Function ' Always have this before your error block.
ErrorHandler:
fnGetNumbers = "An error occurred while extracting numbers from the text."
Exit Function ' While not necessary if
' it is the only error handling block, it can be good practice.
End Function
It is also best to return some kind of value that is useful for debugging. Returning a simple string is useless, whereas returning a value that describes the kind of error is more useful.
Upvotes: 2