Clean code adheres to a number of principles. Does adhering to these principles make good code? Maybe, maybe not. But it definitely helps. One thing I find myself repeating quite a lot in my more recent Stack Overflow answers, is that code should “say what it does, and do what it says” – to me this means writing explicit code. Not just having Option Explicit
specified, but avoiding the pitfalls of various “shortcuts” VBA lets us use to… cheat ourselves.
Avoid implicit member calls, write code that says what it does, and does what it says. Instead of:
Cells(i, 2) = 42
Prefer explicit qualifiers, and explicit member calls:
ActiveSheet.Cells(i, 2).Value = 42
In Excel, avoid working with ActiveSheet when you mean to work with Sheet1. Use the Worksheets collection instead of the Sheets collection when you mean to retrieve a worksheet in a workbook; sheets can contain charts and other non-worksheet sheet types.
Dim targetBook As Workbook Set targetBook = Application.Workbooks.Open(path) Dim targetSheet As Worksheet Set targetSheet = targetBook.Worksheets("Sheet1") Debug.Print targetSheet.Range("A1").Value
If the sheet we need exists in ThisWorkbook
at compile-time, then we don’t need a variable for it – it already exists:
Debug.Print Sheet1.Range("A1").Value
Every sheet in your Excel VBA project has a code name that you can set to any valid VBA module identifier name (up to 31 characters), and that identifier is now accessible from anywhere in your VBA project. To change the name, modify the (Name)
property in the properties toolwindow (F4).
About the Bang! operator…
Avoid the Bang! operator. How many of the people using it know that the identifier to the right of the operator is a string literal that isn’t compile-time validated? It looks like early-bound code, but it isn’t. The Bang! operator is an implicit default member call against a default member that takes a string parameter. So this:
rs.Fields!Field1 = 42
Is really this:
rs.Fields.Item("Field1").Value = 42
Now, this doesn’t mean we have to go crazy and dogmatic here – default properties are idiomatic, and not necessarily toxic… when used carefully. The Item
member of a collection class is, by convention, the default member of the class:
rs.Fields("Field1").Value = 42
Note that Fields
is plural, which strongly signals that ("Field1")
is an indexed property accessor (it is)… and we could even infer that it returns a Field
object reference. There’s an implicit default member call happening, yes, but it’s pure syntax sugar here: even if we don’t know that Fields
is a class with a default Item
property, we can tell that syntactically, we’re invoking something, getting an object reference back and assigning its Value
property with a value.
Contrast with rs.Fields!Field1 = 42
, which reads like… witchcraft, come to think of it.
As an Excel programmer I’m biased though: Access programmers probably see the Bang! operator differently. After all, it’s everywhere, in every tutorial – why would it suddenly be wrong?
Pros:
- Faster to type (?).
- Encourages using standard PascalCase field names and collection keys. Kinda.
Cons:
- Confusing syntax for an unfamiliar reader; makes a string look like a member access. That one’s arguably on the reader/maintainer to read up, yes. Still.
- No compile-time validation: what follows the
!
operator is a string… And any member calls that might follow are always late-bound: Option Explicit will not save you from a typo. - If any explicit member call follows the string, it is inherently late-bound and not compile-time validated either; the editor will offer no intellisense for it.
- Requires otherwise rather uncommon
[square bracket]
tokens around the name when the name contains spaces.
You have to put the Bang! operator in context: 25 years ago, using fully spelled-out variable names was seen as wasteful and borderline ludicrous. Code was written to be executed, not read: the faster you could type, the better. Oh, how things have changed!
Here’s a screenshot from an old, deleted Stack Overflow question about the Bang! operator in… VB.NET:
The Bang! operator is a relic of the past. There’s no reason to use it in modern code, be it in VBA, VB6… or VB.NET.
I’m glad I’ve never seen it in any of the code I’ve reviewed or write. But, in past years when VBA came out, could it be the Bang! operation was taken from cell formulas (referencing a cell in another sheet … =Sheet2!A1)?
LikeLike
Good question, I’d have to dig up the history of that operator to find out… Could be… Although, it’s most predominantly used in ADO/DAO code, and UI code in Access – I’m thinking documentation and various other writings are responsible for its proliferation… kind of like what happened with Systems Hungarian notation.
LikeLike
Please continue with this clean code blog series. I can’t tell you how big of a difference this has made on my code and how I now approach new problems. I’m constantly looking to improve my coding, and this by far has been the most valuable resource!
LikeLiked by 1 person
I tried it and the bang works with or without square brackets,
? rs1!table_name
? rs1![table_name]
Both give me the same result as
? rs1.fields(“table_name”)
LikeLike
Patrick O’Beirne: it works with or without [] unless the name contains invalid characters. For example, if (for some reason!) you have an Access DB table with a column called “_under” you would need
?rs1(“_under”)
or
?rs1![_under]
?rs1!_under will probably give you a compile time Syntax Error.
I used to come across the ! syntax mainly in DAO examples and originally thought it was tied to DAO in some way. Personally, I stick to the (“”) syntax – as far as I know, it will deal with any character in a name except the ” character itself, which you have to double up – e.g. if someone has chosen to name a column abc”def, you have to use (“abc””def”). For the [] syntax, I do not know what the rules for dealing with “[” and “]” in names are, or what types of object might have those characters in their names (for example, I don’t think you can use “[” in Excel sheet names or Access table names, but you might be able to use them in SQL Server table names. Even then, it’s possible that an ODBC driver or OLE DB provider might mangle the name en route.)
LikeLike
You are not being totally explicit here.
What if you have another workbook activated ?
What if you are in another application while this is running ?
…
Usually this is enough …
So instead of: Cells(i, 2) = 42
use:
Application.ThisWorkbook.Worksheets(name).Cells(i, 2)
cheers
LikeLike
If the sheet exists in ThisWorkbook at compile-time, that’s dereferencing a Worksheet object that is already globally accessible; use its code name instead:
Sheet1.Cells(i, 2).Value = 42
The ActiveSheet is always going to be in ActiveWorkbook, so explicitly qualifying with ActiveSheet *is* explicit about what workbook is involved.
As for an Application qualifier, that is a good point, but only relevant if multiple app instances are involved, in which case you qualify with your app variable, not Application. If you write small procedures that take their dependencies as parameters, you’ll be working with Worksheet objects, and where they came from isn’t of any concern to such procedures. Code creating an Application instance and code manipulating worksheets don’t belong in the same procedure.
LikeLike