Handling dialog closure in a VBA user form

Question

I already asked a Code Review question about Working with a new form instance every time.

As far as I have decided, that the most important part of the code is to work with the instance of the form and to learn how to handle them properly, I have remade the code (roughly using the code from @Mat's Mug) in a way that I am looking for a type of a boilerplate for forms, that I will probably use later.

Thus, let's start. I have a form, that looks like this:

Buttons are called btnStart, btnExit and lblInfo.

This is the code behind the frmMain:

Option Explicit

Public Event OnRunReport()
Public Event OnExit()

Private Sub btnRun_Click()
    RaiseEvent OnRunReport
End Sub

Private Sub btnExit_Click()
    RaiseEvent OnExit
End Sub

Private Sub UserForm_QueryClose(CloseMode As Integer, Cancel As Integer)

    End

End Sub

This is the code behind the modMain:

Option Explicit

Private objPresenter   As clsSummaryPresenter

Public Sub MainGenerateReport()

    Call objPresenter.ChangeLabelAndCaption("Starting and running...", "Running...")
    GenerateNumbers

End Sub

Public Sub GenerateNumbers()

    Dim lngLong         As Long
    Dim lngLong2        As Long

    tblMain.Cells.Clear

    For lngLong = 1 To 3000
        For lngLong2 = 1 To 10
            tblMain.Cells(lngLong, lngLong2) = lngLong * lngLong2
        Next lngLong2
    Next lngLong

End Sub

Public Sub ShowMainForm() 'CTRL+E

    If (objPresenter Is Nothing) Then Set objPresenter = New clsSummaryPresenter
    objPresenter.Show

End Sub

This is the code behind the clsSummaryPresenter:

   Option Explicit

    Private WithEvents objSummaryForm As frmMain

    Private Sub Class_Initialize()

        Set objSummaryForm = New frmMain

    End Sub

    Private Sub Class_Terminate()

        Set objSummaryForm = Nothing

    End Sub

    Public Sub Show()

        If Not objSummaryForm.Visible Then
            objSummaryForm.Show vbModeless
            objSummaryForm.lblInfo = "Press Run to Start"
            objSummaryForm.Caption = "Starting"
        End If

    End Sub

    Public Sub Hide()

        If objSummaryForm.Visible Then objSummaryForm.Hide

    End Sub

  Public Sub ChangeLabelAndCaption(strLabelInfo As String, strCaption As String)

    objSummaryForm.lblInfo = strLabelInfo
    objSummaryForm.Caption = strCaption
    objSummaryForm.Repaint

  End Sub


  Private Sub objSummaryForm_OnRunReport()

    MainGenerateReport
    Refresh

End Sub

Private Sub objSummaryForm_OnExit()

    Hide

End Sub

Public Sub Refresh()

    With objSummaryForm
        .lblInfo = "Ready"
        .Caption = "Task performed"
    End With

End Sub

What I am not sure (and I did not understand) was how to handle the red X in a way that there is no error after it. Thus, I have decided to use the End, which seems to work fine.

I have put the Excel file on GitHub. It contains the same VBA code as shown above, I encourage you to first inspect it with macros disabled, as a good security practice with Excel files from the Internet.

Answer 1

I like how your form is no longer concerned with anything other than being a form.

One More Abstraction Level

I'd go one step further, and encapsulate the controls, so that the presenter class doesn't need to know about the implementation details of the form. For example the ChangeLabelAndCaption method knows that there's a Label control on the form, named lblInfo.

The form could expose a property for it instead:

Public Property Get InformationText() As String
    InformationText = lblInfo.Text
End Property

Public Property Let InformationText(ByVal value As String)
    lblInfo.Text = value
End Property

This adds a little layer of abstraction, so that the presenter no longer needs to care what actual controls are on the form, so if you decide to change the Label control for some Frame instead, nothing breaks - your code is now independent of the form's implementation details:

objSummaryForm.InformationText = strLabelInfo

---

For Closure's Sake

Private Sub UserForm_QueryClose(CloseMode As Integer, Cancel As Integer)

    End

End Sub

The End keyword is essentially analoguous to this:

Sure, that will get your form closed, and everything the VBA runtime has been doing will get released. But it's a tiny little wee bit excessive: the End keyword literally ends execution of all VBA code, right there and then.

The QueryClose event is fired just before the form closes, and its parameters give you a handle on the process.

It's just unfortunate that their types are Integer.

The CloseMode parameter tells you how the form is closing; its value is one of the VbQueryClose enum values:

• vbAppTaskManager
• vbAppWindows
• vbFormCode
• vbFormControlMenu
• vbFormMDIForm

The interesting value - the one that means "user clicked the red X button", is vbFormControlMenu. The integer value for it is 0. So you can do this:

If CloseMode = 0 Then

Or better, this:

If CloseMode = vbFormControlMenu Then

And then handle form close yourself.

When the "red X button" is clicked, the form instance is normally destroyed. If the form contains state that the calling code wants to use, if you don't handle QueryClose and the calling code looks like this:

myForm = New MainForm
myForm.Show vbModal
Debug.Print myForm.SomeProperty

Then the last statement will be a runtime error, because myForm is Nothing at that point.

---

The Golden Rule

He who creates an object, shall be responsible for its destruction.

If the form can destroy itself, the calling code is left needing awkward If myForm Is Nothing post-conditions to handle the case where it created an object that decided to hara-kiri.

A cleaner approach is to prevent the form from being able to destroy itself.

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

We set the Cancel parameter to True, thus preventing the destruction of the object, but only when the CloseMode flag indicates that the form was closed from the control menu. If we didn't call the Hide method, the form would simply refuse to close with the "red X button". This is useful for the rare circumstances where you don't want a form to be closable by the user.

---

The QueryClose handler signature would arguably be much clearer like this:

Private Sub UserForm_QueryClose(ByRef CloseMode As VbQueryClose, ByRef Cancel As Boolean)

Unfortunately that signature mismatches the interface declaration, and wouldn't compile. But handling the QueryClose event works exactly as if that's what the signature was.

---

Hungarian Notation

Your code suffers from the "bad" form of Hungarian Notation. While Systems Hungarian is traditionally acceptable for form controls (e.g. lblInformation, txtUserInput, btnOk, etc.; the prefix tells us what type of control we're looking at), using it in actual code for everyday variables, makes things messy:

Dim lngLong         As Long
Private WithEvents objSummaryForm As frmMain
Public Sub ChangeLabelAndCaption(strLabelInfo As String, strCaption As String)

Hungarian Notation was never intended to turn out this way (<~ must read!).

The guy that invented Hungarian Notation meant prefixes to describe the kind of variable, not their type. Apps Hungarian actually does make the code easier to read and understand, and makes wrong code, look wrong:

't: measured in twips
'p: measured in pixels

tArea = tHeight * tWidth 'obviously correct
tArea = pHeight * tWidth 'obviously wrong

Assuming the same context (twips vs pixels), compare to:

'int: integer
'lng: long

lngArea = intHeight * intWidth 'correct?
lngArea = lngHeight * intWidth 'correct?

When you prefix an object variable with obj, or a string with str, an integer with int, lng, or any other type-prefix, you add zero useful information to its identifier, and make the names read awkwardly.

Dim oRange As Range

Stop doing that.