Matter AI | Code Reviewer Documentation

VBA Anti-Patterns Overview

Using Select/Activate in Excel VBA

' Anti-pattern: Using Select/Activate
Sub FormatCells()
    Sheets("Sheet1").Select
    Range("A1:D10").Select
    Selection.Font.Bold = True
    Selection.Interior.Color = RGB(255, 255, 0)
End Sub

' Better approach: Direct object references
Sub FormatCells()
    With Sheets("Sheet1").Range("A1:D10")
        .Font.Bold = True
        .Interior.Color = RGB(255, 255, 0)
    End With
End Sub

Avoid using Select and Activate methods in Excel VBA. These methods are slow, can be affected by user selection, and make code harder to maintain. Instead, use direct object references to manipulate ranges and other objects.

Using Global Variables Excessively

' Anti-pattern: Excessive use of global variables
Public gUserName As String
Public gLastUpdated As Date
Public gDataArray() As Variant

Sub ProcessData()
    ' Using global variables
    gUserName = "John"
    gLastUpdated = Now
    ReDim gDataArray(1 To 100)
    ' Process data...
End Sub

Sub DisplayResults()
    ' Using global variables
    MsgBox "Data processed by " & gUserName & " at " & gLastUpdated
    ' Display results...
End Sub

' Better approach: Pass parameters between procedures
Sub ProcessData()
    Dim userName As String
    Dim lastUpdated As Date
    Dim dataArray() As Variant
    
    userName = "John"
    lastUpdated = Now
    ReDim dataArray(1 To 100)
    ' Process data...
    
    DisplayResults userName, lastUpdated, dataArray
End Sub

Sub DisplayResults(userName As String, lastUpdated As Date, dataArray() As Variant)
    MsgBox "Data processed by " & userName & " at " & lastUpdated
    ' Display results...
End Sub

Avoid excessive use of global variables. They create hidden dependencies between procedures, make code harder to test and maintain, and can lead to unexpected behavior. Instead, pass parameters between procedures and use local variables when possible.

Not Using Option Explicit

' Anti-pattern: Not using Option Explicit
Sub CalculateTotal()
    counter = 1  ' Implicitly declared variable
    totel = 0    ' Misspelled variable name, creates a new variable
    
    Do While counter <= 10
        totel = totel + counter  ' Using the wrong variable
        counter = counter + 1
    Loop
    
    MsgBox "The total is: " & total  ' Refers to a third variable, which is empty
End Sub

' Better approach: Use Option Explicit
Option Explicit

Sub CalculateTotal()
    Dim counter As Integer
    Dim total As Integer
    
    counter = 1
    total = 0
    
    Do While counter <= 10
        total = total + counter
        counter = counter + 1
    Loop
    
    MsgBox "The total is: " & total
End Sub

Always use Option Explicit at the top of your modules to force explicit variable declaration. This prevents typos in variable names from creating new variables, which can lead to hard-to-find bugs.

Using On Error Resume Next Without Proper Error Handling

' Anti-pattern: Using On Error Resume Next without proper handling
Sub ProcessFiles()
    On Error Resume Next
    ' Code that might cause errors
    Kill "C:\Temp\*.tmp"
    MkDir "C:\NewFolder"
    ' Continuing without checking if errors occurred
End Sub

' Better approach: Proper error handling
Sub ProcessFiles()
    On Error GoTo ErrorHandler
    
    Kill "C:\Temp\*.tmp"
    MkDir "C:\NewFolder"
    
    Exit Sub
    
ErrorHandler:
    Select Case Err.Number
        Case 53  ' File not found
            MsgBox "No temporary files to delete.", vbInformation
            Resume Next
        Case 75  ' Path/File access error
            MsgBox "Cannot create folder. It may already exist.", vbExclamation
            Resume Next
        Case Else
            MsgBox "Error " & Err.Number & ": " & Err.Description, vbCritical
    End Select
End Sub

Avoid using On Error Resume Next without checking for errors afterward. This suppresses all errors but doesn’t handle them, which can lead to unexpected behavior. Instead, use structured error handling with On Error GoTo and handle specific error cases appropriately.

Hardcoding Ranges and Values

' Anti-pattern: Hardcoding ranges and values
Sub GenerateReport()
    ' Hardcoded ranges
    Dim data As Range
    Set data = Sheets("Data").Range("A2:F100")
    
    ' Hardcoded column indices
    For i = 2 To 100
        If Sheets("Data").Cells(i, 3) > 1000 Then  ' Column C
            Sheets("Data").Cells(i, 6) = "High Value"  ' Column F
        End If
    Next i
End Sub

' Better approach: Use named ranges and constants
Sub GenerateReport()
    ' Use named ranges
    Dim data As Range
    Set data = Range("SalesData")
    
    ' Use constants for column indices
    Const COL_AMOUNT As Integer = 3  ' Column C
    Const COL_STATUS As Integer = 6  ' Column F
    Const HIGH_VALUE_THRESHOLD As Double = 1000
    
    For i = 2 To data.Rows.Count + 1
        If Sheets("Data").Cells(i, COL_AMOUNT) > HIGH_VALUE_THRESHOLD Then
            Sheets("Data").Cells(i, COL_STATUS) = "High Value"
        End If
    Next i
End Sub

Avoid hardcoding ranges, cell addresses, and magic numbers in your code. Instead, use named ranges, constants, and dynamic range determination. This makes your code more maintainable and adaptable to changes in the worksheet structure.

Inefficient Worksheet Loops

' Anti-pattern: Inefficient worksheet interaction in loops
Sub ProcessData()
    Dim i As Long
    Application.ScreenUpdating = True  ' Default is True
    
    For i = 1 To 10000
        Cells(i, 1).Value = i
        Cells(i, 2).Value = i * 2
        Cells(i, 3).Value = i * 3
    Next i
End Sub

' Better approach: Optimize worksheet interaction
Sub ProcessData()
    Dim i As Long
    Dim data() As Variant
    ReDim data(1 To 10000, 1 To 3)
    
    ' Work with arrays in memory
    For i = 1 To 10000
        data(i, 1) = i
        data(i, 2) = i * 2
        data(i, 3) = i * 3
    Next i
    
    ' Turn off screen updating before writing to worksheet
    Application.ScreenUpdating = False
    Range("A1:C10000").Value = data
    Application.ScreenUpdating = True
End Sub

Avoid inefficient worksheet interactions, especially in loops. Reading from or writing to worksheets is much slower than working with variables in memory. Use arrays to store and manipulate data, then update the worksheet in a single operation. Also, turn off screen updating during intensive operations to improve performance.

Not Using Proper Data Types

' Anti-pattern: Using Variant for everything
Sub ProcessNumbers()
    Dim i As Variant
    Dim sum As Variant
    Dim count As Variant
    
    sum = 0
    count = 0
    
    For i = 1 To 1000
        sum = sum + i
        count = count + 1
    Next i
    
    MsgBox "Average: " & sum / count
End Sub

' Better approach: Use appropriate data types
Sub ProcessNumbers()
    Dim i As Long
    Dim sum As Double
    Dim count As Long
    
    sum = 0
    count = 0
    
    For i = 1 To 1000
        sum = sum + i
        count = count + 1
    Next i
    
    MsgBox "Average: " & sum / count
End Sub

Use appropriate data types instead of relying on Variant. Variants are flexible but consume more memory and can be slower than specific data types. Use Integer or Long for whole numbers, Double for decimal numbers, String for text, and Boolean for true/false values.

Not Using With Blocks for Multiple Property Settings

' Anti-pattern: Repetitive object references
Sub FormatChart()
    Dim cht As Chart
    Set cht = ActiveSheet.ChartObjects(1).Chart
    
    cht.HasTitle = True
    cht.ChartTitle.Text = "Sales Report"
    cht.ChartTitle.Font.Size = 14
    cht.ChartTitle.Font.Bold = True
    cht.ChartTitle.Font.Color = RGB(0, 0, 128)
    cht.Axes(xlCategory).HasTitle = True
    cht.Axes(xlCategory).AxisTitle.Text = "Product"
    cht.Axes(xlValue).HasTitle = True
    cht.Axes(xlValue).AxisTitle.Text = "Sales"
End Sub

' Better approach: Use With blocks
Sub FormatChart()
    Dim cht As Chart
    Set cht = ActiveSheet.ChartObjects(1).Chart
    
    With cht
        .HasTitle = True
        With .ChartTitle
            .Text = "Sales Report"
            With .Font
                .Size = 14
                .Bold = True
                .Color = RGB(0, 0, 128)
            End With
        End With
        
        With .Axes(xlCategory)
            .HasTitle = True
            .AxisTitle.Text = "Product"
        End With
        
        With .Axes(xlValue)
            .HasTitle = True
            .AxisTitle.Text = "Sales"
        End With
    End With
End Sub

Use With blocks when setting multiple properties of the same object. This makes your code more readable and can improve performance by reducing the number of times the object needs to be evaluated.

Using Macros for Simple Formulas

' Anti-pattern: Using VBA for simple calculations
Sub CalculateTotals()
    Dim lastRow As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    
    For i = 2 To lastRow
        Cells(i, 4).Value = Cells(i, 2).Value * Cells(i, 3).Value  ' Price * Quantity
    Next i
End Sub

' Better approach: Use Excel formulas
Sub SetupFormulas()
    Dim lastRow As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    
    ' Set formula once and fill down
    Range("D2").Formula = "=B2*C2"
    Range("D2").AutoFill Destination:=Range("D2:D" & lastRow)
End Sub

Avoid using VBA for calculations that can be done with Excel formulas. Excel’s built-in formulas are optimized for performance and recalculate automatically when data changes. Use VBA for tasks that can’t be easily accomplished with formulas, such as complex logic, file operations, or user interface interactions.

Not Structuring Code into Procedures

' Anti-pattern: Monolithic procedures
Sub DoEverything()
    ' Initialize variables
    Dim lastRow As Long
    Dim i As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    
    ' Format headers
    Range("A1:D1").Font.Bold = True
    Range("A1:D1").Interior.Color = RGB(200, 200, 200)
    
    ' Process data
    For i = 2 To lastRow
        If Cells(i, 3).Value > 100 Then
            Cells(i, 4).Value = "High"
        ElseIf Cells(i, 3).Value > 50 Then
            Cells(i, 4).Value = "Medium"
        Else
            Cells(i, 4).Value = "Low"
        End If
    Next i
    
    ' Create chart
    ' ... 50 more lines of chart creation code ...
    
    ' Print report
    ' ... 30 more lines of printing code ...
End Sub

' Better approach: Modular procedures
Sub Main()
    FormatHeaders
    ProcessData
    CreateChart
    PrintReport
End Sub

Sub FormatHeaders()
    Range("A1:D1").Font.Bold = True
    Range("A1:D1").Interior.Color = RGB(200, 200, 200)
End Sub

Sub ProcessData()
    Dim lastRow As Long
    Dim i As Long
    lastRow = Cells(Rows.Count, 1).End(xlUp).Row
    
    For i = 2 To lastRow
        Cells(i, 4).Value = GetCategory(Cells(i, 3).Value)
    Next i
End Sub

Function GetCategory(value As Double) As String
    If value > 100 Then
        GetCategory = "High"
    ElseIf value > 50 Then
        GetCategory = "Medium"
    Else
        GetCategory = "Low"
    End If
End Function

Sub CreateChart()
    ' Chart creation code
End Sub

Sub PrintReport()
    ' Printing code
End Sub

Break down large, monolithic procedures into smaller, focused procedures and functions. Each procedure should have a single responsibility. This makes your code more readable, maintainable, and reusable.

Not Using Early Binding When Appropriate

' Anti-pattern: Always using late binding
Sub CreateWordDocument()
    Dim wordApp As Object
    Dim wordDoc As Object
    
    Set wordApp = CreateObject("Word.Application")
    wordApp.Visible = True
    Set wordDoc = wordApp.Documents.Add
    
    wordDoc.Content.Text = "Hello, World!"
    wordDoc.SaveAs "C:\Temp\Test.docx"
End Sub

' Better approach: Use early binding when appropriate
' (Requires setting a reference to Microsoft Word Object Library)
Sub CreateWordDocument()
    Dim wordApp As Word.Application
    Dim wordDoc As Word.Document
    
    Set wordApp = New Word.Application
    wordApp.Visible = True
    Set wordDoc = wordApp.Documents.Add
    
    wordDoc.Content.Text = "Hello, World!"
    wordDoc.SaveAs2 FileName:="C:\Temp\Test.docx"
End Sub

Consider using early binding (setting references and declaring specific object types) when working with external applications like Word or Outlook. Early binding provides IntelliSense, compile-time type checking, and better performance. However, late binding (using Object type and CreateObject) can be more flexible when you need to support different versions of the external application.

Not Using Proper String Concatenation

' Anti-pattern: Inefficient string concatenation
Function BuildReport() As String
    Dim result As String
    Dim i As Long
    
    result = ""
    For i = 1 To 1000
        result = result & "Line " & i & ": " & Cells(i, 1).Value & vbCrLf
    Next i
    
    BuildReport = result
End Function

' Better approach: Use StringBuilder pattern
Function BuildReport() As String
    Dim results() As String
    Dim i As Long
    Dim resultCount As Long
    
    ReDim results(1 To 1000)
    resultCount = 0
    
    For i = 1 To 1000
        resultCount = resultCount + 1
        results(resultCount) = "Line " & i & ": " & Cells(i, 1).Value
    Next i
    
    BuildReport = Join(results, vbCrLf)
End Function

Avoid repeatedly concatenating strings in a loop using the & operator, as this creates a new string object each time. Instead, use the StringBuilder pattern (collecting strings in an array and joining them at the end) or use the Join function for better performance with large strings.

Not Using Proper Loop Constructs

' Anti-pattern: Using the wrong loop construct
Sub ProcessItems()
    Dim i As Long
    i = 1
    
    ' Using Do While when For would be more appropriate
    Do While i <= 100
        ' Process item i
        i = i + 1
    Loop
    
    ' Using For when collection-based iteration would be better
    For i = 1 To ActiveSheet.ChartObjects.Count
        ActiveSheet.ChartObjects(i).Chart.ChartType = xlColumnClustered
    Next i
End Sub

' Better approach: Use appropriate loop constructs
Sub ProcessItems()
    Dim i As Long
    Dim cht As ChartObject
    
    ' Use For loop for known number of iterations
    For i = 1 To 100
        ' Process item i
    Next i
    
    ' Use For Each for collection-based iteration
    For Each cht In ActiveSheet.ChartObjects
        cht.Chart.ChartType = xlColumnClustered
    Next cht
End Sub

Use the appropriate loop construct for each situation. Use For loops when you know the number of iterations in advance, For Each loops when iterating through collections, and Do While or Do Until loops when you need to continue until a condition is met.

Not Using Proper Code Comments

' Anti-pattern: No comments or unhelpful comments
Sub ProcessData()
    Dim x As Long
    
    ' Loop through rows
    For x = 2 To 100
        ' Check value
        If Cells(x, 2).Value > 0 Then
            ' Set color
            Cells(x, 2).Interior.Color = RGB(0, 255, 0)
        End If
    Next x
End Sub

' Better approach: Meaningful comments
' Processes sales data and highlights positive values in green
Sub ProcessData()
    Dim rowIndex As Long
    Const FIRST_DATA_ROW As Long = 2
    Const LAST_DATA_ROW As Long = 100
    Const SALES_COLUMN As Long = 2  ' Column B contains sales figures
    
    ' Highlight cells with positive sales values in green
    For rowIndex = FIRST_DATA_ROW To LAST_DATA_ROW
        ' Only process cells with positive sales values
        If Cells(rowIndex, SALES_COLUMN).Value > 0 Then
            Cells(rowIndex, SALES_COLUMN).Interior.Color = RGB(0, 255, 0)
        End If
    Next rowIndex
End Sub

Use meaningful comments to explain why your code does something, not what it does (which should be clear from the code itself). Include comments for procedure purposes, complex logic, and any non-obvious decisions. Also, use meaningful variable and procedure names to make your code self-documenting.

Not Using Proper Workbook and Worksheet References

' Anti-pattern: Implicit workbook and worksheet references
Sub ProcessData()
    ' This uses the active sheet, which might not be what you expect
    Range("A1").Value = "Header"
    Sheets("Sheet1").Range("B1").Value = "Value"
    Workbooks("Book1.xlsx").Sheets("Sheet2").Range("C1").Value = "Result"
End Sub

' Better approach: Explicit workbook and worksheet references
Sub ProcessData()
    Dim wb As Workbook
    Dim wsData As Worksheet
    Dim wsResults As Worksheet
    
    Set wb = ThisWorkbook  ' The workbook containing this code
    Set wsData = wb.Worksheets("Data")
    Set wsResults = wb.Worksheets("Results")
    
    wsData.Range("A1").Value = "Header"
    wsResults.Range("B1").Value = "Value"
End Sub

Use explicit workbook and worksheet references instead of relying on the active workbook or worksheet. This makes your code more reliable and less dependent on the user’s current selection. Use ThisWorkbook to refer to the workbook containing the code, and declare worksheet variables for clarity.

Not Using Proper Application Settings

' Anti-pattern: Not managing application settings
Sub ProcessLargeDataset()
    Dim i As Long
    
    For i = 1 To 10000
        ' Process data with screen updating on (slow)
        Cells(i, 1).Value = i
    Next i
End Sub

' Better approach: Properly manage application settings
Sub ProcessLargeDataset()
    Dim i As Long
    
    ' Save current settings
    Dim screenUpdateState As Boolean
    Dim eventsState As Boolean
    Dim calculationState As XlCalculation
    
    screenUpdateState = Application.ScreenUpdating
    eventsState = Application.EnableEvents
    calculationState = Application.Calculation
    
    ' Optimize settings for performance
    Application.ScreenUpdating = False
    Application.EnableEvents = False
    Application.Calculation = xlCalculationManual
    
    On Error GoTo CleanUp
    
    For i = 1 To 10000
        ' Process data much faster
        Cells(i, 1).Value = i
    Next i
    
CleanUp:
    ' Restore original settings
    Application.ScreenUpdating = screenUpdateState
    Application.EnableEvents = eventsState
    Application.Calculation = calculationState
    
    If Err.Number <> 0 Then
        MsgBox "Error: " & Err.Description
    End If
End Sub

Properly manage application settings like ScreenUpdating, EnableEvents, and Calculation to improve performance during intensive operations. Always save the original settings and restore them when your procedure finishes, even if an error occurs.

Introduction

Getting Started

Product

Patterns

Integrations

Enterprise

VBA