Check errors before function call or within function?

Question

I am curious to know about what is considered to be the best method to check for errors before running a function. Is it best to make a check before the function is called or within the function itself?

For example, a simplified version of what I'm working on involves this Click subroutine:

Private Sub MyButton1_Click()
    For j = 1 to 3
        CreateChart Sheets(j)
    Next j
End Sub

Where the function it calls is defined like so:

Function CreateChart(Sht As Worksheet) As Boolean
    Set ChtObj = Sht.ChartObjects.Add(40, 40, 600, 300)
    Set Cht = ChtObj.Chart
...
End Function

I am dealing with code with multiple modules and many situations where certain checks need to be performed before the function can successfully run. Is it most appropriate to put a check within the loop in the Click sub routine, something like:

If DoesSheetExist(Sheets(j)) Then CreateChart(Sheets(j))

Or best to put it within the function like:

If Not DoesSheetExist(Sht) Then CreateChart = False: Exit Function

Currently I have a little bit of each practice scattered throughout the code and I would like to clean it up. Is it best to run this check outside the function or within?

I would avoid repetitive checks in main code and put it in your Function code. It's simplier to maintain - e.g. if you'd like to change logic of your check, consider what would be the easiest: change logic in entire code or in single function? — Dmitry Pavliv
– Dmitry Pavliv, Commented Apr 11, 2014 at 19:14
In a situation where I have say 4 checks, and I would like to know which issue is causing the problem when the function fails by means of a MsgBox, then sometimes I leave the checks outside of the loops in the main code so that only one notification appears. Is there a way to have the checks within the functions but only report the issues once? — teepee
– teepee, Commented Apr 11, 2014 at 19:19
what you can do is to change return type of your function from Boolean to say Integer and return 1 when there is no issue, -1 when code fails on first check, -2 - when code fails on second check and so on. And in your loop in main code you can add single msgbox for each error "code". — Dmitry Pavliv
– Dmitry Pavliv, Commented Apr 11, 2014 at 19:26
I am a firm believer that every sub/function should have some type of error handling code. As @simoco says you can return values from your functions to: (a) indicate failure and (b) take a different path. Without traps it's less than fun trying to find the culprit if calls are nested four or five levels deep! — Wayne G. Dunn
– Wayne G. Dunn, Commented Apr 11, 2014 at 19:35
@WayneG.Dunn, so you are advocating having checks within and without as well? — teepee
– teepee, Commented Apr 11, 2014 at 21:09

enderland · Accepted Answer · 2014-04-11 19:38:11Z

2

I am dealing with code with multiple modules and many situations where certain checks need to be performed before the function can successfully run. Is it most appropriate to put a check within the loop in the Click sub routine, something like:

Generally I would avoid repeating the same check in your main code if you are going to do it for each.

In a situation where I have say 4 checks, and I would like to know which issue is causing the problem when the function fails by means of a MsgBox, then sometimes I leave the checks outside of the loops in the main code so that only one notification appears. Is there a way to have the checks within the functions but only report the issues once?

It sounds like you have a handful of checks will be the same for each CreateChart method. I would do something like:

Sub mainSub()
    If validateCreateChart Then
        CreateChart
    End If
End Sub

Sub CreateChart()
    On Error Goto errHandler
    'do stuff
    exit sub
    errHandler:
        msgbox  "Unexpected error: " & err.description
End Sub

Function validateCreateChart() As Boolean


    If ConditionOne Then
        validateCreateChart = False
        MsgBox "error condition 1"
        Exit Function
    End If


    If ConditionTwo Then
        validateCreateChart = False
        MsgBox "error condition 2"
        Exit Function
    End If

    'etc

End Function

You don't want to have your logic for errors/prompts scattered everywhere. Especially if you are doing the same checks and will have the same error prompts for each of them.

Don't leave a sub/function without some error handling or you will find yourself regretting it when some "this will never happen" circumstance happens.

edited Apr 11, 2014 at 19:38

answered Apr 11, 2014 at 19:31

enderland

14.2k18 gold badges105 silver badges157 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

teepee Over a year ago

I like this solution. I've got to untangle the web before I can get everything together, but this should work.

enderland Over a year ago

@teepee I have started switching all my checks to be this format. Otherwise, inevitably, I end up with multiple "almost the same but not" sequences of checks. And let me tell you it's a pain when you have to update them :) You might find reading on DRY relevant.

Collectives™ on Stack Overflow

Check errors before function call or within function?

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related