Web Development

10 ways to screw up your VBA code

It's easy to get enticed into cutting corners when you write VBA code -- but that's likely to create problems down the road. Susan Harkins looks at some misconceptions that lead to bad practices, flawed code, and buggy apps.

Writing code can be a creative or mind-numbing task. But whether you love or hate your work, chances are you take shortcuts that you shouldn't. Most shortcuts violate at least one coding best practice rule. They almost all can lead to bugs or erroneous data. My advice: Don't take these shortcuts when writing VBA code!

Here are some misconceptions that lead people to take unfortunate shortcuts. Although a few of them apply specifically to the VBA language or IDE, most of them apply to writing code in general.

Note: This article is also available as a PDF download.

1: I don't need an Else clause

Several VBA statements, If...Then...Else, Select Case, and so on, include an Else clause. This clause follows all of the specific decision-making conditions and it's at the end for a reason. It's the last opportunity to do something. Many developers bypass this last chance thinking it's unnecessary. The conditions are so tightly specified that there's simply no remaining condition to handle. Of course, the logic might be sound, but you should always expect the unexpected.

Including an Else clause is easy and provides an extra layer of error trapping. You might display a generic error so that the user knows that an expected decision or action did not occur. Or you might log it and send an email to the administrator or in-house developer. Just don't let the event pass unnoted. Otherwise, the inaction will work its way into producing erroneous data that might be difficult, even impossible, to troubleshoot. An unexecuted Else clause is better than the alternative.

2: GoTo is a valid statement; I use it often

GoTo is a valid statement, but used incorrectly it produces hard-to-follow code and often hides errors and poor program design. Don't use GoTo as an easy out when you can't think of a better strategy. Do use itwhen you truly need a straightforward method for redirecting the flow. When you reach for GoTo, ask yourself this question: Is there any other way to handle this redirect? If there is -- thinking loops -- don't use GoTo. (I have never needed a GoTo in VBA development. Not one time.)

3: The compiler is a waste of time

Unlike other compilers, the VBA compiler doesn't produce a standalone module you can execute outside Office. Rather, the VBA compiler is actually a syntax checker. Compiling your code is a quick and easy way to catch syntax errors before you actually run the code. Why should you bother? The syntax checker often provides more insightful information about the error than VBA, so you can quickly fix the problem.

4: There won't be any errors to handle

Most developers aren't so arrogant as to really believe their code is perfect, but some do take error handling rather lightly. Error handling is just as important as your design and logic. Don't dismiss it. If anything, be heavy-handed when it comes to error-handling routines. An unhandled error usually means a phone call to you, because the application has ceased to work. Using appropriate error-handling routines, you can:

  • Share information with your users, including instructions for correcting the error immediately.
  • Help the application recover from the error immediately and silently; users will never even know.
  • Track the error so you can fix it.

5: My users will enter the right data

Depending on users to do anything other than crash your application will break your heart. That's not criticism aimed at users, either. Users aren't stupid, but it isn't their job to make sure everything works properly -- that's your job. You can't depend on them to enter the right type of data. That's where validating data comes in. You can use table properties at the basic level, but most likely, you'll end up using a variety of VBA routines to make additional checks. It's probably the single most important task your program will complete, so don't skimp and don't depend on users to not make mistakes. Catch their errors and correct them using validating routines.

6: Naming conventions are for sissies

When you create a variable, it's a good idea to identify it by data type and purpose. Most VBA developers add a three-character prefix, or tag, to identify the data type. For instance, a string data type for storing last names might be strLastName. The prefix identifies the variable's data type, and LastName identifies the variable's purpose. Some developers find this additional tag an unnecessary nuisance, so they don't add them. In some cases, the data type is obvious -- what else would LastName be but a string? Even though adding the tag does take just a wee bit of time, the benefits are worth it:

  • The tag is self-documenting.
  • When debugging or modifying code, you know the variable's data type instantly.
  • Months after the application is in production and you're long gone, the person maintaining the application will find tagged variables much easier to manage.

7: There won't be any null values

Null values have a way of mucking up the works, no matter what precautions you take. In truth, null values can be helpful if you handle them correctly. It all depends on your perspective. However, assuming you don't have to accommodate them because there won't be any is a disaster looking to happen. VBA provides several tools for finding and working with null values:

  • Use IsNull() to determine whether an expression or value is Null. You can't use comparisons, such as var = Null or var <> Null; direct comparisons always return Null (Transact-SQL sometimes returns False).
  • In Access, use Nz() to return a value other than Null when encountering Null.
  • Use the Variant data type if you need to work with Null variables; it's the only data type that can store Null.

8: I'm the only one using the app, so I embedded the password

Passwords and user id values should never be embedded (stored) in code. You might be the only person planned or authorized to use an application, but that doesn't mean a thing. Intentions are worthless. Even if an embedded password makes things easier for you -- and even if users complain about having to enter information to log on -- always prompt users (that includes you) for their user id and password via a dialog.

9: I tested it when I wrote it; everything's fine

Testing a procedure when you write it isn't enough. Developers are usually the worst testers. They simply don't think like users so they don't make the same kind of creative, often downright innovative, requests that users do. You have to put someone other than yourself into a practice production environment. Find a few people who know nothing about the app and dare them to break it. They will.

10: I don't document; I just write code

If you're the only person who will ever modify your application's code, you can probably skip the documentation part -- after all, you're only punishing yourself. However, most developers want to know a little about the code they're modifying if they didn't write it themselves:

  • The routine's purpose/task/goal
  • A short definition of passed values and arguments (if any)
  • An explanation and perhaps even a bit of justification for odd decisions that break good practice rules
  • Who wrote the original code and when; who modified the code and when -- so they can find you and pick your brain, see that you're fired, or see that you get a great big fat raise!


About

Susan Sales Harkins is an IT consultant, specializing in desktop solutions. Previously, she was editor in chief for The Cobb Group, the world's largest publisher of technical journals.

26 comments
lunchbeast
lunchbeast

I know I'm in the minority, but I've never been a big fan of adding a qualifier to your variable to identify its type, especially the primitives. I've seen too much code where it became necessary sometime in the life of the app to change the type (int to float, short to long, etc), which either left you with a misleading name, or required an exhaustive effort to find all the instances, update their names, and test accordingly. I'm all for standards, but this convention has rubbed me the wrong way my entire career.

massonjj
massonjj

You are just about guaranteed that whomever (even yourself) are going to want to change and/or expand what it does at first. If you write it rigidly and sloppy, it will me a whole rewrite to change it. Write it with expandability and flexibility in mind. It will save you time and headache in the future.

Slayer_
Slayer_

That can go wrong in many ways, and makes your code harder to read.

Tony Hopkinson
Tony Hopkinson

anything. VBA has it's own issues of course, but there are gotchas in any language. Getting the fundmamentals of making your code comprehensible always helps.

seanferd
seanferd

Just thinking about it makes my head hurt.

Slayer_
Slayer_

Considering that the GoTo is the most basic error handling. /Sarcasm Usually when I write code, I wrap all functions in On Error GoTo Oops 'inner code, may contain more error handling for specific errors Exit Sub Oops: call msgbox(err.description) 'This is example only, real error message is better 'do cleanup End Sub This way I'll never get an end/debug error.

techrepubliclist
techrepubliclist

... any error message provided by the developer should include - in the window title or in the error message - complete identification of the error provider. I don't know anyone who runs only one application at a time, and there's always the OS, services, et.al. I cannot count the number of times a message box has popped up telling there has been an error - but it doesn't tell me what application is providing the error message! Yeah, as a developer I _know_ where the error resides. But as a user, I have no idea what owns the error. Gee, it takes all of three (3) seconds to identify the message window ... and I'd really like to know whether the OS is complaining about your code, or your code is complaining about my usage!

Slayer_
Slayer_

I'd rather know at a glance a variables type and scope, then to be spared the minor inconvenience of renaming if I have to change it. Which by the way, is frequently handled with "Find and Replace".

Tony Hopkinson
Tony Hopkinson

More people will see it, you don't have to log in to see answers here..... Naff site anyway.

Slayer_
Slayer_

I don't need an expert sex change.

ssharkins
ssharkins

You're right -- could've made the article 11 instead of 10!

Tony Hopkinson
Tony Hopkinson

Usually call the variable or constant it's stored in password as well. Fortunately their code is usually so bad, no one wants to crack it anyway.

dogknees
dogknees

The statement in question is not a "goto", it's part of the syntax of the "on error" statement. If it was truly a separate "goto", it would be legal to write "on error call MySub", but it's not. This might seem a minor issue, but syntax is black and white in the programming world. There are no shades of gray.

massonjj
massonjj

I think all professional VBA programmers would agree that this is the one (and only) exception for the use of "goto".

dutch_gemini
dutch_gemini

I do use "GoTo" in Loops to avoid working with [many levels of] indented branches of especially If's, like in: For Each Item in Collection If Not Item.Condition Then GoTo NextItem NextItem: Next Item

Tink!
Tink!

Seeing as how it's automatically put in the code to handle Errors when you select "Event Procedure" in the Event tab of a field's properties. Edit: Oops my mistake. It doesn't when you do the above (just did it myself lol) but it does automatically put it in when you create a button with a Click Event) /Edit I leave this bit of code in because it does in fact, display a popup with the Error Description when something doesn't go right in your procedure.

ssharkins
ssharkins

No, SinisterSlay, my code's not crap -- sorry. I don't use GoTo. I suppose I would if I needed to, but I don't. I include appropriate error handling too. I suppose I should've been more specific -- using GoTo instead of function calls to direct the flow of traffic. Of course, you need GoTo as part of an On Error statement, but then... I would call that an On Error statement, not a GoTo.

Tony Hopkinson
Tony Hopkinson

it's not the only way to change non-exceptional program flow though. I really missed exceptions when I got lumbered with VB6. The worst habit of all has to be just sticking on error resume next in and crossing your fingers though. My rational is opposite to yours, I want a full app stop for any condition I haven't dealt with, even go right back to the beginning and start again can be an iffy proposition in a complex program. A nice something has gone wring, may be write to a log, and then halt as an outer exception handler is ok. But if things went that bad it got to there, I don't want it to do anthing else, because it's likely to make things worse.

seanferd
seanferd

I know people do stupid things, but i just can't grok why anyone would bother doing that.

Tony Hopkinson
Tony Hopkinson

Rather write a routine to filter the records I want to process than pass that to one to process them.

Slayer_
Slayer_

I do not know what you write, but what I write is 1 or 2 modules among 1000's, my code has no right to make a user lose a days worth of work cause I wrote a bad piece of code or forgot to handle an exception. Worst case, what my code did is now fubar'd, but not the rest of the system.

Tony Hopkinson
Tony Hopkinson

MyPassword = "S3cr3t" Integrated needs set up, and passing it in on the command line is a bit technical. My all time favourite was a guy who did that but then stuffed it in a shortcut because it was 'too difficult' to remember.

Slayer_
Slayer_

I also don't like 1 line If's cause they break the block flow of VBA. IIF's I have no trouble with however.

Tony Hopkinson
Tony Hopkinson

so then it can make a decision on what the failure in 'your' code means to it..