The art of defensive programming


The art of defensive programming

Or how to write code that will be easy to maintain

Article contributed by Jonathan West

The source code that you write has two quite separate purposes.

1.	It is a set of instructions to the computer, telling it to perform a particular task.
2.	It is a description of the task and how you have gone about executing the task, for yourself and/or other programmers.

This article is mainly about the second purpose, which a surprising number of people don't really think about.

If you can't understand a program, then you can't debug it. Even with code that you have written yourself, if you come back to it six months or a year later, you may find yourself wondering “Why on earth did I write that? What was it for?” It doesn't take long to forget the details of a program when you aren't working on it any more. Make life easier for yourself, and write programs as clearly as possible. Also, provide such defences as you can against the possibility that VBA might change between versions of Word.

The following example is based on a real question that came up in the newsgroups in 1999. The questioner wanted a macro that would do the following for a particular set of styles named “AGR1” to “AGR5”:-

1.	Check to see if the style already exists in the document.
2.	If it does, then do nothing.
3.	If it doesn't, then copy it from a central template.

This is a fairly common kind of problem that Word VBA macros are excellent at solving. The following code was suggested in response to the question.

Dim StyleArray As Variant
Dim StyleExists As Boolean
Sub Numbering()
With ActiveDocument
.UpdateStylesOnOpen = False
End With
StyleArray = Array("AGR1", "AGR2", "AGR3", "AGR4", "AGR5")
StyleExists = False
For x = 0 To 4
For Each oStyle In ActiveDocument.Styles
If oStyle = StyleArray(x) Then
StyleExists = True
End If
Next oStyle
If StyleExists = False Then
Application.OrganizerCopy _
Source:="C:\Program Files\Microsoft Office\" & _
"Templates\Final Numbering TDS.dot", _
Destination:=ActiveDocument, Name:=StyleArray(x), _
Object:=wdOrganizerObjectStyles
End If
StyleExists = False
Next x
End Sub

Now, this code does the job, but there are lots of ways in which it could be made easier to read and understand, and less prone to future problems. Remember, source code isn't just there for the computer to execute. It is a message to yourself (or to the programmer who someday may have to come after you and maintain your code) as to what you were trying to achieve. If you make the code easy to read, then that helps you. Anyway, here is a set of improvements that could be made to the code.

1.	There is no need for the Dim statements to be module-level declarations. They aren't needed in any other routine. Therefore they should come after the Sub Numbering() statement, so that there is no confusion with any other routine which might happen to use the same variable names. People tend to re-use favored and meaningful names, so this problem can be significant.
2.	“Numbering” isn't a very meaningful name for the routine. It's always a good idea give a routine a name that clearly describes its purpose. Perhaps ImportAGRStyles would be better.
3.	You should have the “Require Variable Declaration” checkbox set in the VBA Options dialog, so that there is an Option Explicit statement at the start of every module. That way, there is much less chance that VBA will interpret a typographical error in one of your variable assignments as the creation of an entirely new variable of type Variant. This being so, you would need Dim statements for x and oStyle, as Long & Style respectively. See also Why variables should be declared properly.
4.	With–End With statements are a good idea, but there isn't really any need to use them if you are only putting one item in between. If you have two or more lines between them, then yes, great idea, use them whenever you can.
5.	When updating the code at some point in the future, you might want to add another one or two items to StyleArray. It would be quite likely that you would forget (at least initially) to change the limit value of x, so that it reflects the new size of the array. It is much easier to let VBA to remember for you, by using UBound(StyleArray). That will adjust automatically to the number of items you include using the Array function.
6.	A speedup trick. In the For Each oStyle loop, once you find a case where you set StyleExists = True, you don't need to go round the loop any more times. Therefore you can add an Exit For statement to drop you out of the loop.
7.	Using If StyleExists = False Then is not very efficient. Better would be If Not StyleExists Then. In fact, for readability, better still would be to rename the variable StyleIsMissing and swap the True/False values round, so the line can now read If StyleIsMissing Then
8.	You can reset the value of StyleExists (or StyleIsMissing, as we would be renaming it) just once at the start of the loop, rather than once outside and once at the end of the loop. Fewer lines of code means fewer places that bugs can creep in!
9.	I don't trust default properties of objects. It would be too easy for MS to change them between versions of Word and break something in my code. Therefore I would prefer to use oStyle.NameLocal rather than just oStyle when comparing its value with StyleArray(x). We haven't been through enough versions of Word with VBA to find out whether this may be a common problem with MS, though a few things did change between Word 97 and Word 2000. I would simply prefer to minimize the risk, especially as I think it improves readability. Similarly, in the OrganizerCopy command, it would be better to use ActiveDocument.FullName instead of just ActiveDocument.
10.	Indenting the code so that it follows the structure of the If–Then statements and For–Next loops makes it much easier to understand where the loops and branches are. Also, it is a good idea to indent continuations of lines (perhaps by double the normal amount), so that it is quite clear that they are not new instructions. Also, it is common to indent everything except for comments and the lines which begin and end a routine.
11.	When a few lines of code together are needed for a specific task, keep them together, and then put a blank line below to show that you are starting on a new task. That way, the eye more clearly sees what your design is.

That's quite a few comments, they are longer than the code I've been discussing! However, with every line of code that you publish (by which I mean that will ever be seen by anyone other than yourself), you should be able to justify it as being about the best way it could be written. I have learned the need for this kind of coding discipline the hard way, by having to fix extremely nasty and obscure bugs. Also, I have learned that for longer running macros (though this particular macro should be over quickly) every speedup trick in the book is worth applying. The code with these changes would now look like this.

Option Explicit

Sub ImportAGRStyles()

    Dim StyleArray As Variant
    Dim StyleIsMissing As Boolean
    Dim x As Long
    Dim oStyle As Style

    ActiveDocument.UpdateStylesOnOpen = False
    StyleArray = Array("AGR1", "AGR2", "AGR3", "AGR4", "AGR5")

    For x = 0 To UBound(StyleArray)

        StyleIsMissing = True
        For Each oStyle In ActiveDocument.Styles
            If oStyle.NameLocal = StyleArray(x) Then
                StyleIsMissing = False
                Exit For
            End If
        Next oStyle

        If StyleIsMissing Then
            Application.OrganizerCopy _
                    Source:="C:\Program Files\Microsoft Office\" & _
                    "Templates\Final Numbering TDS.dot", _
                    Destination:=ActiveDocument.FullName, _
                    Name:=StyleArray(x), _
                    Object:=wdOrganizerObjectStyles
        End If

    Next x

End Sub

Note that both versions of the routine do exactly the same thing. Maybe the second is marginally faster, but not to an extent that really matters. However, the second is much safer because the layout is clearer, the variables are local, and default properties are not used. It is no harder to write like this, once you get into the habit. For a short macro like this, perhaps it doesn't matter so much, but once you start writing macros, you will probably think of more complex tasks that you can automate with them. Then the “defensive” approach can make a big difference to the length of time it takes to get things right.

There's one other thing to notice. I didn't put any comments in. For a routine that's as simple as this, if the code is written well, there shouldn't be any need for comments. At most, you might want a line or two at the start to describe the purpose of the routine. (You might also have other “standard” comments describing the author and revision level of the routine, but that's another issue altogether.) For a longer routine or a particularly obscure bit of code, it's probably a good idea to include a few extra comments at strategic points. These comments should aim to explain what you are trying to do, rather than how you are doing it. Once you know the what, the how should be clear from the code itself, and so doesn't need duplication.

The art of defensive programming

Recommended further reading.