- Feature Articles
- CodeSOD
-
Error'd
- Most Recent Articles
- Retry Fail
- Office Politics
- Secret Horror
- Not Impossible
- Monkeys
- Killing Time
- Hypersensitive
- Infallabella
-
Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
If Not Frist Then Else MsgBox("Frist!") End If
Admin
It's actually even worse than Remy says: in VBA, if ByRef/ByVal is omitted, the default is ByVal -- so changing Cancel in Report_NoData actually won't change its value in the calling method.
Admin
This is TRWTF.
If this is the code of the business process, then imagine what the business process itself looks like.
Admin
Generating a MsgBox shouldn't cause an error, yes... but you could run out of memory or run into some other condition that does. However, then you probably have bigger concerns than your VBA app at that point since your entire system is probably unstable. Also chances are that second MsgBox to show the error from opening the first isn't likely to succeed, either.
Admin
I have one possible answer for the "why the empty Then block" question - it's likely because the non-programmer who programmed it originally either didn't realize that every IF doesn't also require an ELSE or because they didn't understand boolean expressions enough to know that the NOT operation exists.
It sounds impossible to anyone who actually understands programming, but it's a common problem with Intro to programming students that they're pattern matching their way through a program without actually understanding the individual elements. It's the same with amateurs moving from Excel to VBA - they're generally smart enough to get the logic they want to implement, but without the training in how to implement it they'll come up with some wonderfully creative solutions that are so much harder than the straightforward thing folks with some training will use.
Admin
" that they're pattern matching their way through a program" - Ahhh.. just like ChatGPT [just 20+ years earlier....]
Admin
"Cancel As Integer" being treated as a Boolean is probably not the original developer's fault, but the fault of VBA as this could well be a method signature generated by VBA. VB6 (which VBA comes from) had similar constructs - for example "Form_QueryUnload(Cancel As Integer)" - you had to set Cancel to true to cancel the unload. (Note the default for parameters is by reference, not by value.)
Admin
This strikes a note with me. A few minutes ago I dashed off an email to a client that's a very large manufacturing firm. Parts of their current system that handle inventory, production, sales, etc. is a many-hundred-table SQL Server database that I don't take any issue with. However, before that, someone decided that core processes for their business could be done in Access. Ugh. Their current IT guy is sharp and we're slowly weaning the company off the Access stuff, but he and I are often left scratching our heads wondering how some things were being done. (I even found a lost scheduled task that was doing some of the work.)
Admin
Integer in VBA are pass by value; in other words Cancel = True does absolutely nothing.
Admin
Report_NoData is a Sub because it is an event handler defined in VBA; all event handlers are Sub with reference parameters. When the current Report generates a NoData event message, this Sub gets called to handle it. If an event handler Sub does not exist for a particular event, then the default behaviour (usually nothing) occurs instead.
Admin
I just checked in Excel. The default mode for an integer in VBA is by reference. However, the result of a calculation, even if you put parentheses around the variable name, will be by value, as the value is only stored in temporary calculation space and not in the variable stack nor heap.
Addendum 2023-02-06 13:50: That is to say
will call by reference, but
will call by value, as this is a calculation, even if a very simple one.
Admin
Many years ago the company I worked for acquired some POS software (Point Of Sale, but also the other meaning) written in VB (not VBA). VB has the same problem of letting the "wrong" people write code. This was developed by a guy who mostly dealt with existing POS system installation, configuration, maintenance (think old school cash registers and such, before full computerized POS terminals) and who was clearly not a professional developer by any stretch of the imagination.
As an example, this key (big) WTF: The routine for handling "tendering" (i.e. paying) was all in one file. 36000 lines. It consisted of 4 or 5 functions, one for each type of tender (cash, gift card, credit, debit, ... ). Each of those functions was about 6000 lines. They were nearly identical except for the (maybe) 10-15 lines that were handled differently for each method, so just a pure copy-and-paste, no parameters needed. Ugh.
And then, as I was starting to whip it into shape (sole developer on the project) they cancelled it. :(
Admin
I will defend the "Select but only using the Else case" in the autogenerated error handler. I would guess that the generator puts it there so you can easily add cases for the error numbers you care about; and if you don't, the handler still does something successfully.
Admin
Oops, you are completely right. Turns out not coding in BASIC for a decade and only seeing VB# code in that time, has dimished my ability for remember calling conventions.
Admin
"TRIAL & ERROR is not a programming concept!" one of my professors once exclaimed. During his many years of teaching, he must have seen many WTFs, especially those originated by "I will arbitrarily change the code as long as the compiler throws error messages; if it stops, the computer will do what I expect it to do." It's quite unpleasant to see that this kind of mentality is still alive and being paid for...
Admin
That would be correct for .NET VB but it's not correct for VBA versions of VB. VBA versions of VB did indeed default to ByRef. (In general, you should tune your intuitions for VBA versions of VB based on Fortran, not on C heritage languages.)
Admin
I hate to say it, but everything about this makes sense to me as someone that touches more Access VBA than I'd like. It's not great, but it's better than some business critical stuff I've had to clean up.
Notes: Report_NoData is an event handler. It fires after a report is formatted, but before printing, if there's no data in the bound query. If Cancel is set to true, then the report closes. So this is auto closing if the report has no data and is opened in print preview, which is sensible enough. The empty if clause looks like a copied template. Lazy, but there are reasons to include custom handling if a certain report is opened with arguments (formatting flags, filters, etc).
NoDataMessage is a public routine, which strongly suggests to me that it's a standard message used across reports. It should be a sub, yeah, but that's probably part of whatever generator built the empty routine with the error handling. Note that there's no easy way to get the calling function and module during run time, so those are built into the error message string. Error handling auto-generation also explains the select clause, as it's the easiest way for special handling for error numbers of concern. Error handling is probably unnecessary here, but losing state in the middle of using any complex Access project is a bitch and a half. Wrapping this kind of generic public routine with a handler is a quick way to make sure you don't get full crashes if a report gets opened from somewhere without error handling. Note that in the sample the routine only gets called if the report is opened without arguments, which suggests the report is being opened manually (ie no higher level error catching available). I'm going to defend the bounce back though. It's a more reliable and manageable structure than the alternatives, which involve attempting to skip past the error handling label. The ExitHere label also lets you include any required clean up when things go wrong in more complex routines.
Admin
That can happen with "real" developers also.
Had a colleague that quite often used copy/paste programming duplicating whole segments just because one or a few lines needed changing and in his mind it was easier than adding a parameter to the method to convey the different intents.
And quite predictably, after a few iterations, those identical methods where no longer as identical since bugs sometime only got fixed in one (the others was missed) or new features was only implemented in one path at a time and in the end slightly differently.
And this was a person working full time as a developer and in many regards quite competent, as in he really knew code and could solve problems, he was just a bit unimaginative and clueless, and very much focused on "easiest way right now" with little to no regard for future maintenance.
Admin
Almost as bad as those weird website\freebie software developers of the early\mid 2000's whom wrote awful code and used rubbish freeware to do so and then charged companies a fortune, you know stuff with Apache TCat and awful java flavours and then insisting they had full admin rights (cause nowt worked for them) to SQL Server ENT servers\dbs and writing the queries in the frontend, great for SQL Injection attacks. VB is fine when used correctly