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.