Working with a new form instance every time

Question

I have a form that looks like this:

It is initialized with a Shortcut from the keyboard. It is in a module:

Public Sub ShowMainForm()
    With frmMain
        .Show vbModeless
    End With        
End Sub

The form has a button:

Private Sub btnRun_Click()
    Call MainGenerateReport    
End Sub

The button runs a procedure, called MainGenerateReport in a module.

Public Sub MainGenerateReport() 
    ' other code;
    Call frmMain.MakeLabel
    ' other code;
End Sub

frmMain.MakeLabel changes a label in the form with some information:

Public Sub MakeLabel()
    Dim c           As Long
    Dim r           As Long
    Me.lbInfo.Visible = checkNumbers
    Me.lbInfo.Clear

    If checkNumbers Then
        With Me.lbInfo
            .ColumnCount = 2
            .ColumnWidths = CStr(Me.lbInfo.Width / 1.8 & ";" & Me.lbInfo.Width / 4)

            For r = 0 To 7
                .AddItem
                For c = 0 To 1
                    .List(r, c) = tblInfo.Cells(1 + r, 3 + c)
                Next c
            Next r
        End With
    End If
End Sub

At the end, if I want to close the form, I use the Esc key. I have a button on the form, btnExit and its cancel property is set to True:

Private Sub btnExit_Click()
    Unload Me
End Sub

The problem: According to VBA best practices, I should initialize the form like this (and rewrite my MakeLabel and the btnExit_Click accordingly):

Public Sub ShowMainForm()
    With new frmMain
        .Show vbModeless
    End With        
End Sub

Is that recommended?

Answer 1

Public Sub ShowMainForm()
    With frmMain
        .Show vbModeless
    End With        
End Sub

You don't want that With block, it's redundant.

Public Sub ShowMainForm()
    With New frmMain
        .Show vbModeless
    End With        
End Sub

You don't want to do that with a vbModeless form either - the instance will be destroyed quite immediately after being created.

For an object-oriented approach that uses a modeless form, I'd suggest you make the form a member of a dedicated "presenter" class:

Option Explicit
Private WithEvents summaryForm As frmMain ' <~ notice WithEvents ..I'll get to it.

Private Sub Class_Initialize()
    Set summaryForm = New frmMain
End Sub

Private Sub Class_Terminate()
    Set summaryForm = Nothing
End Sub

Public Sub Show()
    If Not summaryForm.Visible Then summaryForm.Show vbModeless
End Sub

Public Sub Hide()
    If summaryForm.Visible Then summaryForm.Hide
End Sub

Now, this "presenter" class shall be responsible for accessing the form; notice how it already hides the implementation detail of the modeless-ness to the outside world.

When the user clicks the btnRun button, the form itself shouldn't be responsible for anything - so instead of calling MainGenerateReport directly, we'll fire an event to tell the presenter class that it needs to do something about it:

Option Explicit
Public Event OnRunReport()
Public Event OnExit()

Private Sub btnRun_Click()
    RaiseEvent OnRunReport
End Sub

Private Sub btnExit_Click()
    RaiseEvent OnExit
End Sub

That way the form has no dependencies to other modules, and pretty much zero responsibilities.

Back to the presenter, we can handle these events:

Private Sub summaryForm_OnRunReport()
    MainGenerateReport
    Refresh
End Sub

Private Sub summaryForm_OnExit()
    Hide
End Sub

Public Sub Refresh()
    'todo
End Sub

This leaves the problem of MakeLabel/Refresh. I don't know what tblInfo is, but I'm pretty sure it's not the form's concern. Really all it needs is some Range or ListObject that contains whatever information needs to go into that list - and instead of looping to .AddItem and explicitly set each .ListItem, you could use the control's .RowSource property and avoid looping altogether; this CR Q&A shows how.

Once the presenter class is capable of providing a data source for the form's ListBox (you'll have to verify it it actually binds to the source; if that's the case then you won't even need a refresh button, the list would just update itself).

That would be the Refresh implementation.

The last step is to instantiate the presenter; as long as the presenter instance is alive, the encapsulated form instance lives - there's no need to explicitly Unload Me anywhere.

Say you named the class SummaryPresenter, then you could have it exist in global scope:

Option Explicit
Private presenter As SummaryPresenter

Public Sub ShowMainForm() ' macro attached to shortcut key
    If presenter Is Nothing Then Set presenter = New SummaryPresenter
    presenter.Show
End Sub

Since we made the OnRunReport handler call Refresh after MainGenerateReport runs, the procedure no longer needs to call it explicitly. Or, if it does need to (hard to tell with just a little 'other code; comment to work with), then it can do so by calling the Presenter object's Refresh method:

Public Sub MainGenerateReport() 
    ' other code;
    presenter.Refresh
    ' other code;
End Sub

---

This makes your code use the same instance of frmMain all the time, while separating responsibilities into different [class] modules... which isn't much different from working against the default instance in the first place.

But then, everything boils down to why you would want a new instance every time - IMO if the form is a modeless "toolwindow" that you can show/hide while working in Excel, then it's objectively better (more efficient) to avoid initializing it everytime you want to show it... just like it's more efficient to avoid initializing the listbox columns everytime you refresh it ;-)