I've recently learnt about the Model-View-Presenter architectural pattern and I wanted to give it a go with a simple VBA project.

The aim is to retrieve a person's name from a UserForm.

The userform looks like this:

enter image description here

A user should select a team, which updates the name field > Select a name belonging to that team > Hit OK.

The control names are:

  • TeamSelect
  • NameSelect
  • OkButton
  • CloseButton

I'm particularly interested in feedback on my implementation of MVP (or lack of!)

... So here is the code:

GetName Sub - Standard Module: Macros

Public Sub GetName()
    With New NamePresenter
    End With
End Sub

Model - Class Module: NameModel

Private Type TNameView
    Team As String
    Name As String
End Type
Private this As TNameView

Public Property Get Team() As String
    Team = this.Team
End Property

Public Property Let Team(ByVal value As String)
    this.Team = value
End Property

Public Property Get Name() As String
    Name = this.Name
End Property

Public Property Let Name(ByVal value As String)
    this.Name = value
End Property

Public Property Get IsValid() As Boolean
    IsValid = (this.Name <> vbNullString) And (this.Team <> vbNullString)
End Property

View - UserForm Module: NameView

Public Event UpdateNames(ByVal Team As String)

Private Type TNameView
    Model As NameModel
    Data As NameData
    IsCancelled As Boolean
End Type
Private this As TNameView

Public Property Set Model(ByVal Model As NameModel)
    Set this.Model = Model
End Property

Public Property Set Data(ByVal Data As NameData)
    Set this.Data = Data
End Property

Public Property Get IsCancelled() As Boolean
    IsCancelled = this.IsCancelled
End Property

Private Sub TeamSelect_Change()
    this.Model.Team = TeamSelect.value
    RaiseEvent UpdateNames(TeamSelect.value)
End Sub

Private Sub NameSelect_Change()
    this.Model.Name = NameSelect.value
End Sub

Private Sub OkButton_Click()
    If this.Model.IsValid Then
        MsgBox "Something is missing!"
    End If
End Sub

Private Sub CloseButton_Click()
    this.IsCancelled = True
End Sub

Private Sub UserForm_Activate()
    TeamSelect.List = this.Data.Teams
End Sub

Private Sub UserForm_QueryClose(Cancel As Integer, CloseMode As Integer)
    Cancel = True
End Sub

Private Sub PositionForm()
    With Me
        .StartUpPosition = 0
        .Left = Application.Left + (0.5 * Application.Width) - (0.5 * .Width)
        .Top = Application.Top + (0.5 * Application.Height) - (0.5 * .Height)
    End With
End Sub

Presenter - Class Module: NamePresenter

Private WithEvents View As NameView

Private Model As NameModel
Private Data As NameData

Public Sub Present()
    Set View = New NameView
    Set Model = New NameModel
    Set Data = New NameData

    With View
        Set .Model = Model
        Set .Data = Data
        .Show vbModal
        If .IsCancelled Then Exit Sub
        MsgBox Model.Team & " / " & Model.Name
    End With
End Sub

Private Sub View_UpdateNames(ByVal Team As String)
    View.NameSelect.List = Data.Names(Team)
End Sub

Data - Class Module: NameData

Public Property Get Teams() As Variant
    Teams = Array("Team1", "Team2", "Team3")
End Property

Public Function Names(ByVal Team As String) As Variant
    Select Case Team
        Case "Team1"
            Names = Array("Name1", "Name2", "Name3")
        Case "Team2"
            Names = Array("Name4", "Name5", "Name6")
        Case "Team3"
            Names = Array("Name7", "Name8", "Name9")
    End Select
End Function

Note: I've ommitted Option Explicit only in this post to keep the code-section as short as possible

The UpdateNames event should be TeamChanged or something, so that it tells the presenter what happened on the view, as opposed to what the presenter should be doing; "update names" sounds more like a method name than an event, too.

The presenter shouldn't know about any form controls: the view knows about the model - there's no need to round-trip to the presenter to update the list of names, IMO that's pure presentation logic that should remain encapsulated in the view.

The IsValid logic could be used to keep the OK button disabled until the model is in a valid state; that way you wouldn't need that MsgBox, as the user wouldn't even be able to attempt to submit incomplete/invalid data.

Invoking frmView.Show (i.e. bypassing the presenter) raises run-time error 91 here:

Private Sub UserForm_Activate()
    TeamSelect.List = this.Data.Teams '<< this.Data is Nothing
End Sub

Surely there must be a more elegant way to make the calling code avoid using the form's default instance.

  • 2
    Hi Piotr, the code should work as it is otherwise it shouldn’t be on Code Review. Thanks for taking a look - I’ll check the code again myself when I get a chance and if it actually doesn’t work I’ll remove the question – CallumDA Oct 11 at 11:22
  • TeamSelect is the name of the team drop down box. I provided a list of these names in the original question :) – CallumDA Oct 11 at 11:36
  • Yeah, I named the ComboBox the same: – Piotr Polit Oct 11 at 11:57
  • 1
    You're probably having the form running the show (e.g. UserForm1.Show), which is exactly what model-view-presenter is preventing. The presenter is responsible for showing the form, and setting the model reference (this.Data being Nothing is why you're getting error 91). That said this isn't a troubleshooting site, answers are supposed to be reviewing the OP's code, not asking to clarify how it works. – Mathieu Guindon Oct 11 at 12:04
  • As I said above, you need to use the presenter, not the form's default instance (which I'm assuming is what you're doing, since you're not showing how the form is being created). This article, and this article, should help understand what's going on and why. – Mathieu Guindon Oct 11 at 12:09

protected by Mathieu Guindon Oct 11 at 11:56

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.