garroad_ran
garroad_ran

Reputation: 174

Excel VBA Error Handling Code

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

Answers (3)

Coder375
Coder375

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

brettdj
brettdj

Reputation: 55672

Suggest that you wouldn't run the ErrorHandler section by default for this, ie either

  • get a valid answer then exit the function
  • get a error (tested with 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

Brandon Barney
Brandon Barney

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

Related Questions