VBA UI design architecture, how to make this a piece of art

Question

I have this very recurrent pattern at my VBA macros, where I have to ask the user to give me certain information that will be necessary for the processes to come. Most often for example, I need him to give me some workbooks and maybe a date.

So I often declare a Type whose elements are all the workbooks I'm going to need, which will be the parameter I'll be passing around the processes. In this case, like this (variable names have been deliberately changed to hide the privacy of the actual report I'm making here):

Public Type FourTrackers
 aFirst As Workbook
 bSecond As Workbook
 cThird As Workbook
 dFourth As Workbook
End Type

A userform with fields will collect strings with the fullpath to the workbooks, and perhaps some dropdown lists with possible months and years for

Option Explicit
'####################
' Always in this order
' Constructor & Destructor
' OK & Cancel buttons
' QueryClose & frmUserCancelled
' Relevant Type getter
' Form Fields events
' Helper Functions
'####################
'Private module level data variable
Private mCancel As Boolean
Private FourTrackers As FourTrackers
'@Description("Enumeration of colours. Keep in mind that VBA spells a colour as BGR (inverse order).")
Private Enum meColours
 good = &HFFFFFF
 bad = &HD9BFB
End Enum
'@Description("Initiate the form listing values for month and year, and marking the boxes as unempty")
Private Sub UserForm_Initialize()
 mCancel = False
 Static iSingleton As Long
 iSingleton = iSingleton + 1
 If iSingleton > 1 Then Err.Raise 1000, mainForm, "This userform is a singleton!"
 'files
 Me.T_First.BackColor = meColours.bad
 Me.T_Second.BackColor = meColours.bad
 Me.T_Third.BackColor = meColours.bad
 Me.T_Fourth.BackColor = meColours.bad
End Sub
'@Description("")
Private Sub UserForm_Terminate()
 Debug.Print "Termination of the userForm. Remember it should be a singleton, this shouln't appear"
 'Debug.Assert False
End Sub
'@Description("")
Private Sub btn_OK_Click()
 If Me.T_First.BackColor = meColours.good _
 And Me.T_Second.BackColor = meColours.good _
 And Me.T_Third.BackColor = meColours.good _
 And Me.T_Fourth.BackColor = meColours.good _
 Then
 FourTrackers.aFirst= Workbooks.Open(Me.T_First.Value)
 FourTrackers.bSecond= Workbooks.Open(Me.T_Second.Value)
 FourTrackers.cThird= Workbooks.Open(Me.T_Thirds.Value)
 FourTrackers.dFourth= Workbooks.Open(Me.T_Fourth.Value)
 Me.Hide
 Else
 MsgBox "Select first all reports"
 End If
End Sub
'@Description("")
Private Sub btn_Cancel_Click()
 mCancel = True
 Me.Hide
End Sub
'@Description("in case of queryclose, remit yourself to a cancel as well.")
Private Sub UserForm_QueryClose(Cancel As Integer, CloseMode As Integer)
 If CloseMode = vbFormControlMenu Then
 Cancel = True
 btn_Cancel_Click
 End If
End Sub
'@Description("Indicate if the form has been canceled for real. Good for QueryClose event.")
Public Property Get frmUserCanceled() As Boolean
 frmUserCanceled = mCancel
End Property
'@Description("Returns the struct with the Trackers")
Public Property Get frmFourTrackers() As FourTrackers
 frmFourTrackers = FourTrackers
End Property
' double click on the box or on the dots will have the same effect
Private Sub T_First_DblClick(ByVal Cancel As MSForms.ReturnBoolean)
 B_First_Click
End Sub
Private Sub T_Second_DblClick(ByVal Cancel As MSForms.ReturnBoolean)
 B_Second_Click
End Sub
Private Sub T_Third_DblClick(ByVal Cancel As MSForms.ReturnBoolean)
 B_Third_Click
End Sub
Private Sub T_Fourth_DblClick(ByVal Cancel As MSForms.ReturnBoolean)
 B_Fourth_Click
End Sub
' Select the files
Private Sub B_First_Click()
 Dim temp As String
 temp = selectFiles("First", ThisWorkbook.Path, False, "Excel files", "*.xlsx, *.xls, *.csv, *.xlsb, *.xlsm")
 Me.T_First.Value = temp
 Me.T_First.BackColor = auxSelectColor(temp)
 Call FillFromTheFirst(temp)
End Sub
Private Sub B_Second_Click()
 Dim temp As String
 temp = selectFiles("Second", ThisWorkbook.Path, False, "Excel files", "*.xlsx, *.xls, *.csv, *.xlsb, *.xlsm")
 Me.T_Second.Value = temp
 Me.T_Second.BackColor = auxSelectColor(temp)
End Sub
Private Sub B_Third_Click()
 Dim temp As String
 temp = selectFiles("Third", ThisWorkbook.Path, False, "Excel files", "*.xlsx, *.xls, *.csv, *.xlsb, *.xlsm")
 Me.T_Third.Value = temp
 Me.T_Third.BackColor = auxSelectColor(temp)
End Sub
Private Sub B_Fourth_Click()
 Dim temp As String
 temp = selectFiles("Fourth", ThisWorkbook.Path, False, "Excel files", "*.xlsx, *.xls, *.csv, *.xlsb, *.xlsm")
 Me.T_Fourth.Value = temp
 Me.T_Fourth.BackColor = auxSelectColor(temp)
End Sub
'@Description("Helper function. Prompts the user to select a file and returns a string with its whole path and fullname.")
Private Function selectFiles(ByVal DisplayText As String, _
 ByVal InitialFolder As String, _
 Optional ByVal MultipleSelection As Boolean = False, _
 Optional ByVal filter As String = vbNullString, _
 Optional ByVal filterType As String = vbNullString _
 ) As String
 Dim fDialog As FileDialog
 Set fDialog = Application.FileDialog(msoFileDialogFilePicker)
 With fDialog
 'Optional: FileDialog properties
 .AllowMultiSelect = MultipleSelection
 .Title = DisplayText
 .InitialFileName = InitialFolder
 'Optional: Add filters
 .Filters.Clear
 .Filters.Add filter, filterType '??
 'Show the dialog. -1 means success!
 If .Show = -1 Then
 selectFiles = fDialog.SelectedItems(1)
 Else
 selectFiles = vbNullString
 End If
 End With
End Function
'@Description("Selects a colour, good if the string parameter is not empty, bad if vbNullString.")
Private Function auxSelectColor(ByVal temp As String) As meColours
 If temp <> vbNullString Then
 auxSelectColor = meColours.good
 Else
 auxSelectColor = meColours.bad
 End If
End Function
' Select the files
Private Sub FillFromTheFirst(ByVal temp As String)
 'just spagetthi code to try to predict the other three workbooks once we know where the first one is, so we don't have to subsequently ask the user for a lot of prompts to select files.
End Sub

As I've tried, the UI will only verify that the four files have been selected on the OK button, and if so, assign the workbook references to the struct and then hide himself; so far this is the "only" interaction of the UI with the Data, which I've tried to separate as much as I can. But here is where I see all this UI Architectural Patterns appearing on my Stack Overflow reseach and I'm not sure of the approach to take.

Then, this is the call on a standard module:

Public mainForm as mainForm 'does this shuts up the creation of the default UserForm?
Public Type TwoSapReports
 ReportMain As Workbook
 ReportSubTotals As Workbook
End Type
Public Sub DataForm(ByRef rep As TwoSapReports)
 Dim myTrackers As FourTrackers
 With New mainForm
 .Show
 If Not .frmUserCanceled Then
 meTrackers = .frmFourTrackers
 End If
 .Hide
 End With
 'do stuff with myTrackers.
End Sub

Answer 1

Public mainForm as mainForm 'does this shuts up the creation of the default UserForm?

Nope. It merely declares a global variable that's never assigned or referred to. I'd just remove it, its presence isn't justified; it's shadowing the global-scope default instance.

That makes code like this:

mainForm.Show

Throw runtime error 91 because the reference isn't Set and an unqualified mainForm would be referring to that global variable, but it doesn't prevent accessing it through fully-qualified calls:

VBAProject.mainForm.Show

In other words, it's nothing but a minor hurdle that makes the reader raise an eyebrow - it creates more problems than it solves.

A word about this:

Public Type TwoSapReports
 ReportMain As Workbook
 ReportSubTotals As Workbook
End Type

I don't think that Type is justified, or wise: UDT's represent values; I tend to see them like C# struct or COM TKIND_RECORD instances - [ab]using as lazy substitutes for class modules / objects is a bad idea IMO. You will inevitably run into issues as you pass the UDT value around, and/or need to use it elsewhere in the project.

---

This is a lie:

Static iSingleton As Long
iSingleton = iSingleton + 1
If iSingleton > 1 Then Err.Raise 1000, mainForm, "This userform is a singleton!"

The only way to implement a true Singleton in VBA is to have the class exist in another project, PublicNotCreatable with a factory method that handles instantiation - something that's definitely overkill, at least in the context of a UserForm.

A better solution is to write your UserForm so that it works fine regardless of whether it's an instance, or the default instance.

Trying to control how a VBA class is used, is stepping on the toes of the calling code.

If you really want to do that, you can do it without involving a Static counter, with the Is operator:

If Me Is VBAProject.mainForm Then
 ' code is running against the default instance
End If

The problem is that putting that check in the UserForm_Initialize handler will always trip the check, because UserForm_Initialize will run at least once on the default instance, the first time the class is used.

So the best place to put that check is in the UserForm_Activate handler instead:

Private Sub UserForm_Activate()
 If Me Is VBAProject.UserForm1 Then MsgBox "Hello from default instance!"
End Sub

Form instances involve mechanics that are outside of your control as a VBA programmer - there's no point trying to interfere with them, and if your form needs to care about how it was instantiated, then it's doing too many things: static code analysis isn't a form's job - telling the programmer they're misusing a form's default instance is the job of a static code analyzer, such as Rubberduck - an IDE add-in. These things belong in the IDE itself, not in your code.

I see you are using Rubberduck:

'@Description("Helper function. Prompts the user to select a file and returns a string with its whole path and fullname.")

Kudos for these - however @Description annotations are very much akin to XML-doc and docstrings in other languages: having them on Private members is next to useless. I like that all Public members have a description though.

This is noise:

'@Description("")

This isn't nice:

Private Sub UserForm_QueryClose(Cancel As Integer, CloseMode As Integer)
 If CloseMode = vbFormControlMenu Then
 Cancel = True
 btn_Cancel_Click
 End If
End Sub

You're "manually" invoking a Click event handler procedure: that's rather dirty. I would extract a private method from the click handler:

Private Sub OnCancelled()
 Me.Hide
 mCancel = True
End Sub

And then invoke that private method in both the Click and QueryClose handlers.

The Ok button's Click handler feels wrong too - it's doing way too much work. This is what I'd expect an Ok button to do:

Private Sub OkButton_Click()
 Me.Hide
End Sub

That's absolutely all of it. What happens when the dialog is okayed (or cancelled) should be in the hands of whatever code invoked the form's Show method, not the form itself.

See UserForm1.Show (a recent article I wrote) for more thorough information about what the respective roles of a form and its calling code are.

• Nelson Vides: right, I was expecting something like this. You're totally right on the structs, it's just a lazy object. I'll improve that. On the code checking within the OKbutton, I was imagining it was too much doing for an UI, this should be handled elsewhere. But I'm not sure... maybe just declaring a property that would get the object I wanted to extract, and calling this property from the outside? Then, where should I put the setter for the object I want to return?
• Mathieu Guindon: @NelsonVides See the linked article - it describes how to separate the Model from the View, which is what you're trying to do here.