I don’t know if these are really “best practices” or if I’m just making them up. I know I heard them somewhere, but that doesn’t make them universal by any means. Also, I generally agree with all of these best practices, although I have no intention of changing my ways.
Best Practice:Never use lowercase ‘i’ or lowercase ‘l’ as variables.
Reasoning:Both look too much like the number 1 and makes the code very difficult to debug.
Justification:I’ve been bitten by the
i problem myself. But
k will always be my
For Next control variables. I’ve accepted the fact that I’m never going to change that.
Best Practice:Don’t reuse variables
Reasoning:When you reuse variables, you have to be careful to reinitialize the variable and you won’t be as careful as you think. Also, how could you have a well-named variable that you use in two places?
Justification:If I’m coding two loops in a procedure, I reuse the control variable. I think that’s a reasonable exception because it’s a control variable and it will initialize itself in the loop. However, I’m probably also guilty of using a variable like
lCnt in two places. That’s one of those generically named variables that should really identify what it’s counting. If it was
lWorkbookCnt, then it would be a lot harder to justify reusing it later when counting worksheets.
Best Practice:Align data types in Dim statements. Like
Dim wb As Workbook
Dim MySheet As Worksheet
Dim lCnt As Long
Reasoning:The block of declared variables is easier to read.
Justification:I can’t imagine a worse fate than having to adjust all of my Dim statements when I add a new variable whose name is longer than the rest.
Best Practice:Declare each variable on its own line
Reasoning:It prevents the problem of not assigning a data type. A variable declaration like
Dim sFile, sPath As String
is really declaring sFile as a Variant (because omitted data types are Variants) and sPath as a String. By giving each variable its own line, you are less likely to make that mistake.
Justification:Bah. I’m a careful programmer. Not unusually careful, just averagely careful, and I don’t make that mistake. The only people who make that mistake are people that don’t understand that you have to declare a data type for every variable on the same line. Once you know how it works, it’s just not a mistake you’re going to make. Most of my variables are on their own line, but closely related variables often end up sharing a line. In my code you’ll often see
Dim i As Long, j As Long, k As Long
Dim sFile As String, lFile As Long
(although let’s hope I don’t have three nested For Each loop too often)
Best Practice:Comment, comment, comment
Reasoning:You and everyone else that reads your code will find it easier to understand if you include comments.
Justification:Most comments suck. If 25% of your code lines are comments, you and everyone else that reads your code will skip over them anyway (the VBE makes them a different color so that it’s super easy to skip over them). This best practice stems from the fact that most people don’t comment enough. In reality, most people don’t write readable enough code, which is far superior to commenting. When I’m writing code to teach a beginner, I comment every line. But teaching is different. If you feel you need a comment, consider rewriting your code. If you consider it and still need a comment, you probably really need a comment.
Best Practice:Avoid Exit For and Exit Do in loops
Reasoning:Exit For and Exit Do are just like GoTo. And we all know that GoTo is the spawn of Satan.
Justification:I abhor GoTo so deeply that I want to be on the other side of this argument. But I can’t justify continuing a loop when I’ve found what I need. Here’s a crappy example of what I mean.
For lCnt = 1 To colLocations.Count
If colLocations.Item(lCnt) = "Omaha" Then
bOmahaExists = True
Of course I would never hardcode a string or have domain specific data in a variable name, but you get the point. The main differences between Exit For/Do and GoTo is that the Exit statements only flow in one direction (only forward, while GoTo can go forward or backward) and the Exit statement flow to a specific place (the line below Next or Loop, while GoTo can go wherever you choose).
Best Practice:Use named parameters
Reasoning:Named parameters make the code more readable and don’t enforce an order to the parameters, reducing bugs.
Set rFound = rRng.Find(What:="stuff", LookIn:=xlValues, LookAt:=xlWhole)
Set rFound = rRng.Find("stuff", , xlValues, xlWhole)
Justification:I hate named arguments. They just clutter everything up and make it ugly. I want my code to be pretty. I don’t absolutely value prettiness over readability, but I don’t absolutely discount it either. I don’t need to write code for a complete beginner to read (unless I know that’s my audience). If you are reading my code, you should have a pretty good working knowledge of the Find method and what it’s arguments are. If you’re reading this post and don’t have the exact order of the parameters memorized, you probably could figure it out by the constants used. And failing that, you could just look it up. Which is what you should do if you don’t know. Don’t write code for your non-programmer supervisor to read – he won’t get it even with named arguments and copious comments.
If there is a best practice you knowingly avoid, leave a comment. You don’t have to justify it. There is no justification for using ‘i’ as a variable, but I still do it.