Thomas Jefferson's Accidental Programming Advice

Mike Wolfe -
The government closest to the people serves the people best.

This Jeffersonian principle also applies to variable declaration. Mutatis mutandis:

The variable declared closest to the code serves the code best.

This is all an unnecessarily highfalutin way to say STOP DECLARING ALL OF YOUR VARIABLES AT THE TOP OF YOUR PROCEDURES!!!!

But don't take my word for it.

Code Complete to the Rescue

Let's refer to what I consider the authoritative text on practical programming advice, Code Complete: Second Edition, by Steve McConnell.

Here's an extended excerpt covering this topic from pages 241 - 242 of the book:

10.3 Guidelines for Initializing Variables

Improper data initialization is one of the most fertile sources of error in computer programming. Developing effective techniques for avoiding initialization problems can save a lot of debugging time.  The problems with improper initialization stem from a variable’s containing an initial value that you do not expect it to contain.

...

Following are guidelines for avoiding initialization problems:

...

Initialize each variable close to where it’s first used Some languages, including [classic] Visual Basic, don’t support initializing variables as they’re declared. That can lead to coding styles like the following one, in which declarations are grouped together and then initializations are grouped together—all far from the first actual use of the variables.

Example of Bad Initialization

' declare all variables
Dim accountIndex As Integer
Dim total As Double
Dim done As Boolean

' initialize all variables
accountIndex = 0
total = 0.0
done = False
...

' code using accountIndex
...

' code using total
...

' code using done
While Not done
   ...

A better practice is to initialize variables as close as possible to where they’re first used:

Example of Good Initialization

Dim accountIndex As Integer
accountIndex = 0
' code using accountIndex
...

Dim total As Double
total = 0.0
' code using total
...

Dim done As Boolean
done = False
' code using done
While Not done
   ...

The second example is superior to the first for several reasons. By the time execution of the first example gets to the code that uses done, done could have been modified. If that’s not the case when you first write the program, later modifications might make it so. Another problem with the first approach is that throwing all the initializations together creates the impression that all the variables are used throughout the whole routine—when in fact done is used only at the end. Finally, as the program is modified (as it will be, if only by debugging), loops might be built around the code that uses done, and done will need to be reinitialized. The code in the second example will require little modification in such a case. The code in the first example is more prone to producing an annoying initialization error.

This is an example of the Principle of Proximity: keep related actions together. The
same principle applies to keeping comments close to the code they describe, keeping loop setup code close to the loop, grouping statements in straight-line code, and to many other areas.

I'm Sorry, Microsoft, But You Are Wrong

Seemingly every VBA example Microsoft publishes reinforces this bad habit.

Case in point:

Sub GetProductStats() 
 
Dim dbsNorthwind As DAO.Database 
Dim rstProducts As DAO.Recordset 
Dim rstCategories As DAO.Recordset 
Dim varFirstMark As Variant 
Dim varHighMark As Variant 
Dim varLowMark As Variant 
Dim curHighRev As Currency 
Dim curLowRev As Currency 
Dim strSQL As String 
Dim strCriteria As String 
Dim strMessage As String 
 
On Error GoTo ErrorHandler 
 
   Set dbsNorthwind = CurrentDb 
 
   strSQL = "SELECT * FROM Products WHERE UnitsOnOrder >= 40 " & _ 
            "ORDER BY CategoryID, UnitsOnOrder DESC" 
   Set rstProducts = dbsNorthwind.OpenRecordset(strSQL, dbOpenSnapshot) 
   If rstProducts.EOF Then Exit Sub 
 
   StrSQL = "SELECT CategoryID, CategoryName FROM Categories " & _ 
            "ORDER BY CategoryID" 
   Set rstCategories = dbsNorthwind.OpenRecordset(strSQL, dbOpenSnapshot) 
 
   ' For each category find the product generating the least revenue 
   ' and the product generating the most revenue. 
   Do Until rstCategories.EOF 
 
      strCriteria = "CategoryID = " & rstCategories![CategoryID] 
      rstProducts.FindFirst strCriteria 
      curHighRev = rstProducts![UnitPrice] * rstProducts![UnitsOnOrder] 
 
      If Not rstProducts.NoMatch Then 
 
         ' Set bookmarks at the first record containing the CategoryID. 
         varFirstMark = rstProducts.Bookmark 
         varHighMark = varFirstMark 
         varLowMark = varFirstMark 
 
         ' Find the product generating the most revenue. 
         Do While rstProducts![CategoryID] = rstCategories![CategoryID] 
            If rstProducts![UnitPrice] * rstProducts![UnitsOnOrder] > _ 
            curHighRev Then 
               curHighRev = rstProducts![UnitPrice] * _ 
                            rstProducts![UnitsOnOrder] 
               varHighMark = rstProducts.Bookmark 
            End If 
            rstProducts.MoveNext 
         Loop 
 
         ' Move to the first record containing the CategoryID. 
         rstProducts.Bookmark = varFirstMark 
         curLowRev = rstProducts![UnitPrice] * rstProducts![UnitsOnOrder] 
 
         ' Find the product generating the least revenue. 
         Do While rstProducts![CategoryID] = rstCategories![CategoryID] 
            If rstProducts![UnitPrice] * rstProducts![UnitsOnOrder] < _ 
            curLowRev Then 
               curLowRev = rstProducts![UnitPrice] * _ 
                           rstProducts![UnitsOnOrder] 
               varLowMark = rstProducts.Bookmark 
            End If 
            rstProducts.MoveNext 
         Loop 
 
      End If 
 
      ' Set high and low bookmarks to build the message string. 
      strMessage = "CATEGORY:  " & rstCategories!CategoryName & _ 
                   vbCrLf & vbCrLf 
      rstProducts.Bookmark = varHighMark 
      strMessage = strMessage & "HIGH: $" & curHighRev & "  " & _ 
                   rstProducts!ProductName & vbCrLf 
      rstProducts.Bookmark = varLowMark 
      strMessage = strMessage & "LOW:  $" & curLowRev & "  " & _ 
                   rstProducts!ProductName 
      MsgBox strMessage, , "Product Statistics" 
      rstCategories.MoveNext 
   Loop 
 
   rstProducts.Close 
   rstCategories.Close 
   dbsNorthwind.Close 
 
   Set rstProducts = Nothing 
   Set rstCategories = Nothing 
   Set dbsNorthwind = Nothing 
 
Exit Sub 
 
ErrorHandler: 
   MsgBox "Error #: " & Err.Number & vbCrLf & vbCrLf & Err.Description 
End Sub

Here's what it looks like if we move the declarations to the lines just before we reference them.  Also, because the variables are declared so close to where they get used, the already-atrocious Systems Hungarian prefixes (e.g., var, cur, str, etc.) truly serve no purpose now, so I removed them entirely.  Here's the rewritten code in my preferred style:

Sub GetProductStats()
    On Error GoTo ErrorHandler
    
    Dim dbsNorthwind As DAO.Database
    Set dbsNorthwind = CurrentDb
    
    Dim SQL As String
    SQL = "SELECT * FROM Products WHERE UnitsOnOrder >= 40 " & _
             "ORDER BY CategoryID, UnitsOnOrder DESC"
    
    
    Dim rstProducts As DAO.Recordset
    Set rstProducts = dbsNorthwind.OpenRecordset(SQL, dbOpenSnapshot)
    If rstProducts.EOF Then Exit Sub
    
    SQL = "SELECT CategoryID, CategoryName FROM Categories " & _
        "ORDER BY CategoryID"
    Dim rstCategories As DAO.Recordset
    Set rstCategories = dbsNorthwind.OpenRecordset(SQL, dbOpenSnapshot)
    
    ' For each category find the product generating the least revenue
    ' and the product generating the most revenue.
    Do Until rstCategories.EOF
    
        Dim Criteria As String
        Criteria = "CategoryID = " & rstCategories![CategoryID]
        rstProducts.FindFirst Criteria
        Dim HighRev As Currency
        HighRev = rstProducts![UnitPrice] * rstProducts![UnitsOnOrder]
    
        If Not rstProducts.NoMatch Then
    
            ' Set bookmarks at the first record containing the CategoryID.
            Dim FirstMark As Variant, HighMark As Variant, LowMark As Variant
            FirstMark = rstProducts.Bookmark
            HighMark = FirstMark
            LowMark = FirstMark

            ' Find the product generating the most revenue.
            Do While rstProducts![CategoryID] = rstCategories![CategoryID]
                If rstProducts![UnitPrice] * rstProducts![UnitsOnOrder] > _
                    HighRev Then
                    HighRev = rstProducts![UnitPrice] * _
                        rstProducts![UnitsOnOrder]
                    HighMark = rstProducts.Bookmark
                End If
                rstProducts.MoveNext
            Loop
    
            ' Move to the first record containing the CategoryID.
            rstProducts.Bookmark = FirstMark
            Dim LowRev As Currency
            LowRev = rstProducts![UnitPrice] * rstProducts![UnitsOnOrder]
    
            ' Find the product generating the least revenue.
            Do While rstProducts![CategoryID] = rstCategories![CategoryID]
                If rstProducts![UnitPrice] * rstProducts![UnitsOnOrder] < _
                    LowRev Then
                    LowRev = rstProducts![UnitPrice] * _
                        rstProducts![UnitsOnOrder]
                    LowMark = rstProducts.Bookmark
                End If
                rstProducts.MoveNext
            Loop
    
        End If
    
        ' Set high and low bookmarks to build the message ing.
        Dim Message As String
        Message = "CATEGORY:  " & rstCategories!CategoryName & _
            vbCrLf & vbCrLf
        rstProducts.Bookmark = HighMark
        Message = Message & "HIGH: $" & HighRev & "  " & _
            rstProducts!ProductName & vbCrLf
        rstProducts.Bookmark = LowMark
        Message = Message & "LOW:  $" & LowRev & "  " & _
            rstProducts!ProductName
        MsgBox Message, , "Product Statistics"
        rstCategories.MoveNext
    Loop
    
    rstProducts.Close
    rstCategories.Close
    dbsNorthwind.Close
    
    Set rstProducts = Nothing
    Set rstCategories = Nothing
    Set dbsNorthwind = Nothing

    Exit Sub
 
ErrorHandler:
    MsgBox "Error #: " & Err.Number & vbCrLf & vbCrLf & Err.Description
End Sub

I Get By With a Little Help From My Compiler

Since we are all using Option Explicit around here (that's a statement, not a question), we gain some additional help from the compiler by declaring our variables closer to where we use them.

Why You Should Always Use Option Explicit in VBA
Don’t let typos and logic errors ruin your VBA code. Read our latest blog post to learn about the importance of Option Explicit and how to use it.
No Longer SetMike Wolfe

With Option Explicit in place, our code will not compile if we try to use a variable before we declare it.  The declaration line must precede its first use.  Perhaps that's why people started declaring variables at the top of procedures in the first place.

But refactoring your code to avoid compile errors is counterproductive.  It's not so much that compile errors are good, it's just that compile errors are way better than runtime or logic errors.

Suppose we have some code that uses a counter to break out of a loop:

Sub MyProcedure()

    'Do a bunch of stuff

    Dim i As Long
    Do Until i >= MaxLoops
        i = i + 1
        'Do some other stuff
    Loop
End Sub

In the above code, we don't explicitly initialize i, so it gets the default Long value of 0.

If, years later, we try to initialize i to something other than zero in the portion of the code where we are "doing a bunch of stuff," we will get a compile error, "Variable not defined":

Sub MyProcedure()

    'Do a bunch of stuff
    i = 42
    'Keep doing stuff

    Dim i As Long
    Do Until i >= MaxLoops
        i = i + 1
        'Do some other stuff
    Loop
End Sub

A compile error in this situation is good!  It alerts us to the fact that we are doing something unexpected.  It's easy to resolve the compile error: either we move the Dim i As Long line up above the line of code that sets i = 42 or we create a new variable to hold the secret to life, the universe, and everything (i.e., 42).

On the other hand, if we had declared Dim i As Long in the procedure header (following Microsoft's poor example), this unexpected code change could have easily gone unnoticed.  At best it would have been a runtime error.  At worst, it would have been a festering logic error.