Simple MVP Name-Finder

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:

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
.Present
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
Me.Hide
Else
MsgBox "Something is missing!"
End If
End Sub

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

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

Private Sub UserForm_QueryClose(Cancel As Integer, CloseMode As Integer)
Cancel = True
CloseButton_Click
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()
PositionForm
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.

• 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 '18 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 '18 at 11:36
• Yeah, I named the ComboBox the same: – Piotr Polit Oct 11 '18 at 11:57
• 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 '18 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 '18 at 12:09

protected by Mathieu GuindonOct 11 '18 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).