Search code examples
vbaexcel

Global variable value in Worksheet_Calculate() event handler is always incorrect


I have a problem with a global variable whose value is not always correct.

I have declared it like this:

Public NumRows As Long

It is used to hold the number of used rows on Sheet 1. The variable is public because multiple macros use it.

It is initialised here:

Private Sub Workbook_Open()
  NumRows = Worksheets("TableSize").Range("A1").Value
  MsgBox "NumRows = " & NumRows
End Sub

MsgBox was inserted simply to verify that the code was working. Sheet TableSize only has information in A1. At this point, NumRows = 32 which is correct.

This is the event handler in Sheet TableSize that is causing problems:

Private Sub Worksheet_Calculate()    

    Dim n As Long
    n = Worksheets("TableSize").Range("A1").Value 'A1 contains the formula "=ROW(INDEX(Sheet1,1,1))+ROWS(Sheet1)-1" used to count rows on Sheet 1
    MsgBox n & "NumRows=" & NumRows
    If n = NumRows Then Exit Sub
    If n > NumRows Then Call NewDatabaseEntry

    NumRows = n    

End Sub

MsgBox returns 32 for n like it should, but 0 for NumRows, even though NumRows was correctly set in the Workbook_Open() handler!

The desire is for n to always equal NumRows unless I actually add a row, so that only then will NewDataEntry (correctly) create a new data entry on another sheet.

Am I missing something crucial?


Solution

  • Actually, the solution is very simple. It involves two parts.

    The first part solves your immediate issue. All you need to do is move the NumRows global variable declaration from the "ThisWorkbook" module (where it seems like you have it now) to a standard module. (The "Module1" module you mentioned in a previous question is fine.)

    Your Workbook_Open() event handler will still be able to access the variable, as will code in every other module, and everything should still work (or not if there are other issues) just as it currently does.

    The second part addresses the more general problem of why what you did didn't cause an error. It will also help avoid a lot of other issues in the future. Just as Storax has suggested, although I will word it a bit stronger, you must use the Option Explicit statement at the top of every code module.

    The best way to do this is set up the VBIDE to do so automatically. Go to Tools > Options… > Editor and check the Require Variable Declaration option. (Make sure you click the OK button ;-) )

    From now on, Option Explicit will automatically be added to all new modules that you create, and to all the modules in new workbooks that you create. However, existing modules will need to be manually updated.


    Another good idea, according to the DRY principle, would be to place the table size code in a function. Together with a bug-fix, your code would look like this:

    ' In any standard module
    Option Explicit
    
    Public NumRows As Long
    
    Public Function GetTableSize() As Long
      GetTableSize = Worksheets("TableSize").Range("A1").Value2
    End Function
    
    ' In the "ThisWorkbook" module
    Option Explicit
    
    Private Sub Workbook_Open()
      NumRows = GetTableSize()
    End Sub
    
    ' In the "TableSize" sheet module
    Option Explicit
    
    Private Sub Worksheet_Calculate()
    
        Dim n As Long
        n = GetTableSize()
        If n > NumRows Then NewDatabaseEntry
        ' Always set NumRows so that even after entries are deleted (n < NumRows),
        ' adding new entries will work correctly.
        NumRows = n
    
    End Sub
    


    An even better idea would be to dispense with the "TableSize" helper worksheet and its Worksheet_Calculate() event handler, and instead use the Worksheet_Change() handler of Sheet 1 to directly detect added rows. (Not the Worksheet_Calculate() handler that you previously attempted to use.)