2
\$\begingroup\$

What will be the best practice to create an efficient error handler in object class in VBA? Creating some for Outlook (used later in both, Excel and Access) and trying to get the most efficient and well-built handler. My concern here is that I have multiple Functions returning an object which makes bubble structure harder to implement.

In Class

 Public  sOutClassErrStr As String
 Private Function OutMain() As Object
 On Error Resume Next
 Set OutMain = GetObject(, "Outlook.Application")
 If Err.Number = 429 Then
    Set OutMain = CreateObject("Outlook.Application") 'Handled error - no reason to raise.
 ElseIf Err.Number <> 0 And Err.Number <> 429 Then
    sOutClassErrStr = sOutClassErrStr & Err.Number & ";" & Err.Description & ";" & "cOutlookFunctions.OutMain" & ";" & Date & " " & Time & "|"
End If
End Function

Private Function OutNmspc() As Object
On Error Resume Next
Set OutNmspc = OutMain.GetNamespace("MAPI")            'Outlook.Namespace -- only MAPI is available, at least MS Docs say so
If Err.Number <> 0 Then sOutClassErrStr = sOutClassErrStr & Err.Number & ";" & Err.Description & ";" & "cOutlookFunctions.OutNmspc" & ";" & Date & " " & Time & "|"
End Function

Private Function OutRecip(ByVal sUserResolve As String) As Object
On Error Resume Next
Set OutRecip = OutMain.session.CreateRecipient(sUserResolve)
OutRecip.Resolve
If Err.Number <> 0 Then sOutClassErrStr = sOutClassErrStr & Err.Number & ";" & Err.Description & ";" & "cOutlookFunctions.OutRecip" & ";" & Date & " " & Time & "|"
If Not OutRecip.Resolved Then sOutClassErrStr = sOutClassErrStr & "1" & ";" & "Recipient not resolved." & ";" & "cOutlookFunctions.OutRecip" & ";" & Date & " " & Time & "|"    'custom error - raise by dev
End Function

Private Function GetFolderObject(ptyFolder As enOutFolder, Optional ByVal sUserName As String = "", Optional ByVal sShareMB As String = "") As Object
If sShareMB <> "" Then
    On Error Resume Next
    Set GetFolderObject = OutMain.GetNamespace("MAPI").Folders(sShareMB).Folders("Inbox")
    If Err.Number <> 0 Then
        MsgBox "You don't have an access/mapped " & sShareMB & " mailbox.", vbExclamation
        Exit Function
    End If
End If
If sUserName <> "" Then 
'To add err handling when username not exist like 'asdasda.dasdasdd'
    Set GetFolderObject = OutNmspc.GetSharedDefaultFolder(OutRecip(sUserName), ptyFolder).Items
    Exit Function
End If
Set GetFolderObject = OutNmspc.GetDefaultFolder(ptyFolder).Items
End Function

First, even though it's widely criticized, thanks for that.
To clarify a couple of things though. In the end, I want to use this class in, as mentioned, only in Excel/Access projects. I did not copy the whole class into the code, but just a sample how I want to refer inside/outside of it.
There are public Sub's like SendEmail, AddAppointemt, ShareCalendar etc. Mentioned functions are the 'guts' of the publics. This string, though not sure whether it should be public, as I'm thinking of it now, is displayed in ErrHandler UDF created separately. And, in the end, I want to display it - of course, delimited to columns - whenever something goes wrong atop.
At the same time want to avoid raising too many errors while the reason is one i.e.

Private Function App As Object
On Error Resume Next
Set App = GetObject(,SomeApp)
If Err.Number <> 0 Then strErr = strErr &  Err.Desc  & "|"
End Function
Private Function ChildApp As Object
On Error Resume Next
If strErr <> "" Then Exit Fucntion
Set ChildApp = App.Items
If Err.Number <> 0 Then strErr = strErr &  Err.Desc  & "|"
End Function
Public Sub DisplayNoOfItems
If strErr <> "" Then frmErrHandler(strErr).Show
MsgBox "There are " & ChildAp.Count & " items."
End Sub

Where in this form code will be terminated after errors are being handled. Don't want to receive info about errors in both of the Priv Functions as, which is obvious, in most cases, if one function throws an error, the second one also won't be working fine.

About naming convention - you're right, I've just shortened Hungarian notation --> I'm referring to sSomeString or cSomeClass. It's easier for me. The same goes for adding 'Out' in the begining of my Outlook priv function.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ At a quick glance: If Err.Number = 429 Then ... ElseIf Err.Number <> 0 And Err.Number <> 429 Then ... there's no need to check that Err.Number <> 429 in the ElseIf. You'll never get to the ElseIf if it does = 429 - that'll be handled in the If. \$\endgroup\$
    – FreeMan
    Commented Apr 4, 2019 at 17:11
  • \$\begingroup\$ Welcome to CR! Can you edit your post to include example usage of this class? How is your program instantiating it, and how is it using sOutClassErrStr? Also, unclear how this relates to "bubbling", given On Error Resume Next and no Err.Raise going on anywhere... \$\endgroup\$ Commented Apr 4, 2019 at 18:10
  • \$\begingroup\$ When I dumped your first block into a Class module and second into a Standard module I get a compile error "User-defined type not defined" at Private Function GetFolderObject(ptyFolder As enOutFolder, Optional ByVal sUserName As String = "", Optional ByVal sShareMB As String = "") As Object. Am I missing something? \$\endgroup\$ Commented Apr 9, 2019 at 13:01

1 Answer 1

2
\$\begingroup\$

I don't think this should be a class: it's a collection of very loosely related functions (they all do something with Outlook... beyond that...), but the only instance-level piece of data is this Public sOutClassErrStr string, which is constantly being appended to.

This is not a good or idiomatic approach to sane error handling, and is mixing concerns - conflating things like logging (presumably that's what that string is for?) and providing user feedback (MsgBox), and not bubbling any errors in the process.

No matter what you do, error state is going to be global, with Err.Description giving you the description for the current error, wherever you are in the project: exception made of vbWatchDog (no affiliation, but its author did contribute to Rubberduck, an open-source VBIDE add-in project I contribute to), every single attempt ever made at changing how error handling works in VBA (or VB6), has been doomed, cumbersome, bug-prone, confusing, and/or just plain wrong.

The function names are unclear and confusing. What is "out main" supposed to be doing? Looks like it's essentially a factory method to get/create an Outlook.Application instance. But, if this code is used in Outlook, then the global-scope Application identifier already holds a reference to that very object we're trying to get. Why the Out prefix though? Because "Outlook"? The class is already named cOutlookFunctions, so why have every single one of its members remind us "this is for Outlook!" ...if we didn't want Outlook functions, we wouldn't be using an cOutlookFunctions class in the first place!

That said, consider using TypeName(Me) to get the name of the current class, rather than hard-coding it.

Rule of thumb, avoid prefixes. They are useless and clutter up the code, making it harder to read than necessary. c-for-class serves no purpose, s-for-string either. Avoid disemvoweling, too. There is no reason to turn Namespace into Nmspc, other than to make it impossible to pronounce. Worse, if you don't code against the Outlook object model all that often, you might not even remember what it stands for when you get back to that code 6 months from now -- be it you, or your successor.

The effect of returning Nothing instead of bubbling errors up, is that you're taking the real source of the error, burying it in some string (whose value is lost forever once execution ends), and then forcing the caller to immediately check whether it got Nothing, otherwise they might very well be throwing error 91 at a completely unrelated place, well further down the execution path, far away from the source of the problem.

What you're trying to do, is much better handled with a "try" pattern.

Public Function TryGetSomething(ByVal thing As String, ByRef outResult As Object) As Boolean

    Dim result As Boolean
    On Error GoTo TryFailed

    Set outResult = Things(thing)
    result = True

CleanExit:
    TryGetSomething = result
    Exit Function

TryFailed:
    result = False
    Set outResult = Nothing
    Resume CleanExit
End Function

Note the out prefix stands for "output" and is meaningful in this case - unlike an "out" prefix meaning "outlook", which is redundant. Structured like this, the client code can intuitively do:

Dim something As Object
If TryGetSomething("thing", outResult:=something) Then
   'try succeeded: [something] holds the object reference we needed.
Else
   'try failed: write to log, pop MsgBox, etc.
End If

But, I'd probably move all these TryXxxxx functions to a standard module - there's no need to have a class to only hold methods that don't work with any instance state, especially if the class is meant to be a single-instance object.

As for the sOutClassErrStr "log" string, I'd remove it entirely, and use an actual logger class instead, so that the logs are actually persisted or at least output to the immediate pane as soon as the entries are written - that way the information won't be lost if the host application crashes or execution is accidentally ended.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.