5

I'm trying to refactor some Excel VBA code (Excel 2016, VBA 7.1) to eliminate repetitive subroutines for maintainability purposes. Several of the subroutines only differ by the group of global constants they use, so what I'm basically trying to do is group the globals together into a struct-like data structure so I can pass them into a common function as an argument.

Note that the groups of global constants have some in common, but not all, e.g.:

Global Const GROUP1_SHEET As String = "Sheet1"
Global Const GROUP1_FIRST_ROW As Long = 2
Global Const GROUP1_LAST_COL As Long = 15
Global Const GROUP1_SOME_COL_OFFSET = 4

Global Const GROUP2_SHEET As String = "Sheet2"
Global Const GROUP2_FIRST_ROW As Long = 2
Global Const GROUP2_LAST_COL As Long = 8
Global Const GROUP2_ANOTHER_COL_OFFSET = 2

And there are different subroutines for each group, e.g.:

In Sheet1:

Private Sub DoSomething()
    Set w = Worksheets(GROUP1_SHEET)
    'some code
End Sub

In Sheet2:

Private Sub DoSomething()
    Set w = Worksheets(GROUP2_SHEET)
    'same code as above
End Sub

There are dozens of these. Needless to say, this code is a nightmare to even read through, let alone maintain.

What I'm trying to do right now is split the groups into separate modules and set them as properties, similar to what is described in this question. The problem with this is I don't know how to pass the module (i.e. the group of globals) to the function as an argument.

In new module GROUP1:

Public Property Get SHEET() As String
    SHEET = "Sheet1"
End Property

And this works as I want it to:

Public Sub ShowPopup()
    MsgBox GROUP1.SHEET
End Sub

But passing it as an argument does not:

Public Sub Popup(inModule As Object)
    MsgBox inModule.SHEET
End Sub

Public Sub ShowPopUp()
    Popup GROUP1
End Sub

Nothing I've tried works in place of "Object" in the example above. I either get "ByRef Argument type mismatch" or "Expected variable or procedure, not module" depending on what I put there.

So, can I pass a module like this (maybe as a string and evaluate it somehow?), or should I use some other way of grouping globals?

user13803
  • 95
  • 6
  • 2
    Make it a `Class Module` instead of a simple `Module` and you will be able to do that. See http://www.cpearson.com/excel/classes.aspx or https://stackoverflow.com/q/2161666/8597922 – Victor K Aug 07 '19 at 14:38
  • Thanks! Since it's now a class module, it has to be instantiated, and since objects can't be global constants, I have to Dim and Set the object in ShowPopUp to pass it in, right? If you made that an answer I'd accept it. – user13803 Aug 07 '19 at 16:26
  • Have a look at [CallByName](https://learn.microsoft.com/en-us/office/vba/language/reference/user-interface-help/callbyname-function) to invoke a method using a string [Passing a property as an argument for userform controls](https://stackoverflow.com/questions/48952665/passing-a-property-as-an-argument-for-userform-controls/48952944#48952944). I don't know if your desigm is recommened, but you should ask at https://codereview.stackexchange.com/ – ComputerVersteher Aug 18 '19 at 15:07

1 Answers1

4

You cannot pass regular modules as arguments (technically you can pass a string and use Application.Run, but that might be a nightmare to deal with), but you can pass classes.

Classes can be in global scope. So technically you can instantiate it at some point (opening a workbook for example) and then use them at any point. I would say that globals are fine in some cases, but most of the time you can (and perhaps should) do without them. I would encourage you to look at the topic of global scope and why it often considered to be bad.

You can have a class like this:

GroupClass:
Option Explicit
Private Type TypeGroup
    WS as WorkSheet
    FirstRow as Long
    FirstCol as Long
    ColumnOffset as Long
End Type
Private This as TypeGroup
Public Function Initialize(Byval WS as Sheet, Byval FirstRow as Long, ByVal FirstCol as Long, ByVal ColumnOffset as Long)
With This
    Set .WS = WS
    .FirstRow = FirstRow
    .FirstCol = FirscCol
    .ColumnOffset = ColumnOffset
End with
End Function
Public Property Get Name() as String
    Name = This.WS.Name
End Property

Then you can use it like this:

Public Sub Popup(Group As GroupClass)
    MsgBox Group.Name
End Sub

Public Sub ShowPopUp()
    Dim Group1 as GroupClass
    Set Group1 = New GroupClass
    Group1.Initialize Worksheets("Sheet1"),2,15,4
    Popup Group1
End 

The reason I use Private Type and Private This as TypeGroup can be found here. Some caveats for class instantiation can be seen here. Depending on what you do, your class organization can be very different. You can read on interfaces, immutability, when to use getters/setters, encapsulation and other topic elsewhere.

Victor K
  • 1,049
  • 2
  • 10
  • 21
  • Thanks. I'm trying to stick to a light-touch approach for this refactoring effort, just re-organizing the ~850 global constants and simplifying/commonizing functions. I realize it won't be proper, but it should be much quicker for me to complete and easier for the original author and other users to follow that way. Hopefully then it will also be easier to flush out some of the odd behavior we're seeing. That's the plan, at least... – user13803 Aug 07 '19 at 17:42