Visual Basic for Applications represents the core mistake of putting a full-featured programming environment on every desktop. That so much VBA code is bad is not remarkable- that any good code exists would be shocking.
We rarely cover VBA code, because most of it is written by a non-programmer who discovered they could solve real business problems in Microsoft Access. TRWTF is, in fact, how much of the world runs on an Access database stuffed into a network share somewhere. But there are organizations that hire developers and then shove them into writing VBA, which is what happened to Doug. This code comes from quite awhile ago.
Doug inherited a bunch of VBA code written by another developer. It has… issues. One issue, which Doug didn't send us, was a 500 switch statement followed by a 700 line switch statement where the vast majority of the clauses were duplicated between the two, and also all the bodies were simply setting a boolean variable to true or false.
That's ugly and awful, but what Doug sent us is just weird:
Private Sub Report_NoData(Cancel As Integer)
If Me.OpenArgs = True Then
Else
NoDataMessage
End If
Cancel = True
End Sub
This is a weird little block of code. The empty if
clause, for example, is weird. Why? Why is Cancel
being set to true all the time? Why was it specified as an Integer
if that's not what it holds? What exactly is NoDataMessage
doing? Let's look at that.
Public Function NoDataMessage()
' Code Header inserted by VBA Code Commenter and Error Handler Add-In
'=============================================================
' modGeneral.NoDataMessage
'-------------------------------------------------------------
' Purpose
' Author : Joebob Bobsen, Tuesday, November 05, 2002
' Notes :
'-------------------------------------------------------------
' Parameters
'-----------
'
'-------------------------------------------------------------
' Returns:
'-------------------------------------------------------------
' Revision History
'-------------------------------------------------------------
' Tuesday, November 05, 2002:
'=============================================================
' End Code Header block
On Error GoTo HandleErr
MsgBox "No data was returned by this report", vbInformation, GetOption("Application Name")
ExitHere:
Exit Function
' Error handling block added by VBA Code Commenter and Error Handler Add-In. DO NOT EDIT this block of code.
' Automatic error handler last updated at Tuesday, November 05, 2002 2:12:06 PM
HandleErr:
Select Case Err.Number
Case Else
MsgBox "Error " & Err.Number & ": " & Err.Description, vbCritical, "modGeneral.NoDataMessage"
End Select
Resume ExitHere
' End Error handling block.
End Function
The core of this method is just to pop up a message box: "No data was returned by this report". Nothing about popping up the message box could cause an error, and while GetOption
might if you try and access an invalid option in Access (the docs are vague on this), Application Name
is a valid option, so that line of code couldn't cause an error.
It doesn't matter, though, because we handle the error anyway, thanks to an auto-generated error handler. Which I know we shouldn't get too picky about auto-generated code, but the whole thing gets wrapped up in a switch statement (select
, in VBA parlance) to only have the default case.
And while the VB-style error handling of using gotos for flow control is terrible, this method adds an extra layer by bouncing back up to a label for the sole purpose of exiting the function. Which, speaking of weirdness: why is this a function in the first place? It doesn't return a value. Report_NoData
should be a function, since it generates an output (through a reference parameter), but NoDataMessage
is not a function, it does not return a value.
This code is confusing, perplexing, and sadly, was absolutely vital to a business process for a number of years. Clearly, someone knew the code was bad and wanted to fix it, and thus brought in code quality tools, like the "VBA Code Commenter and Error Handler Add-In", but just adding exception handling isn't enough to make the code any better, especially if the exception handling is in the wrong place.
This post originally appeared on The Daily WTF.