Working with a new form instance every time

Question

I have a form that looks like this:

Screenshot

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?

• Mathieu Guindon: Why does MainGenerateReport need to call frmMain.MakeLabel? What's the other code doing?
• Vityata: MainGenerateReport is uniting a few excel files together. In the frm.MakeLabel, there is information about the total number of rows of the newly created file and other relevant information, for the operation carried out. Something like Successfully carried out, 1000 rows are added etc.
• fernando.reyes: Would that work if you pass the form as a parameter to the other functions byref, and then access directly the instance you created with new? like MainGenerateReport(ByRef main as Form)
• Vityata: @fernando.reyes - for MakeLabel it would probably work. However, it will not work at the end, when the form stays and displays information. Then it is closed manually by the user - either through the X or through the escape key, which runs currently with btnExit_Click.
• fernando.reyes: @Vityata The way to close every windows from a macro is with the End statement. The End statement will close everything you had open, close files, etc. msdn.microsoft.com/en-us/library/0wt87xba.aspx

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 ;-)

• Vityata: Impressive! Thanks, I wIll take a better look at it tomorrow :)
• Vityata: Hi again! :) I have ran it and it seems that sometimes there is an error. The error is in Public Sub Show of clsSummaryPresenter` On the line -> If Not objSummaryForm.Visible Then. It is because the code checks an object for visibility, which is nothing. The screenshots are the two *.png files in the repository here - github.com/Vitosh/VBA_personal/tree/master/…. I am trying to build some kind of boilerplate for a form for some future projects.
• Vityata: Actually, I found out when exactly it crashes - if I close the form with the X on the top right corner, the next initialization breaks with the errors I have shown.
• Mathieu Guindon: You still need to handle QueryClose and prevent the form from unloading itself.
• Vityata: Yeah, QueryClose is what I need. I have put the following in clsSummaryPresenter, but it does not even enter the code - pastebin.com/NQ68Bpgn