patternMinor
Down the rabbit hole with MVP
Viewed 0 times
thewithmvpdownholerabbit
Problem
Following-up on this post where I implemented a Repository Pattern in vba to abstract ADODB and enable testing my Excel app without hitting a database backend; curious about how far vba would let me push inversion-of-control and loose coupling, I decided to grab the red pill, and see how deep the rabbit hole goes.
Presentation
With the original design, despite the decoupled data access, I still would have needed a form for every table I wanted to maitain, since the presentation logic was still coupled with the views.
I scratched that, and created a
SimpleView form
```
Option Explicit
Private Type ViewModel
Model As SqlResult
Selection As SqlResultRow
Callback As ICommandCallback
End Type
Private vm As ViewModel
Private minHeight As Integer
Private minWidth As Integer
Private layoutBindings As New List
Implements IView
Private Sub UserForm_Initialize()
minHeight = Me.Height
minWidth = Me.Width
BindControlLayouts
End Sub
Private Sub BindControlLayouts()
Dim backgroundImageLayout As New ControlLayout
backgroundImageLayout.Bind Me, BackgroundImage, AnchorAll
Dim closeButtonLayout As New ControlLayout
closeButtonLayout.Bind Me, CloseButton, BottomAnchor + RightAnchor
Dim itemsListLayout As New ControlLayout
itemsListLayout.Bind Me, ItemsList, AnchorAll
Dim addButtonLayout As New ControlLayout
addButtonLayout.Bind Me, AddButton, RightAnchor
Dim editButtonLayout As New ControlLayout
editButtonLayout.Bind Me, EditButton, RightAnchor
Dim showDetailsButtonLayout As New ControlLayout
showDetailsButtonLayout.Bind Me, ShowDetailsButton, RightAnchor
Dim deleteButtonLayout As New ControlLayout
deleteButtonLayout.Bind Me, DeleteButton, RightAnchor
layoutBindings.Add closeButtonLayout, _
bac
Presentation
With the original design, despite the decoupled data access, I still would have needed a form for every table I wanted to maitain, since the presentation logic was still coupled with the views.
I scratched that, and created a
SimpleView form that I intended to use for anything I would want to maintain with that application, now or in the future.SimpleView form
```
Option Explicit
Private Type ViewModel
Model As SqlResult
Selection As SqlResultRow
Callback As ICommandCallback
End Type
Private vm As ViewModel
Private minHeight As Integer
Private minWidth As Integer
Private layoutBindings As New List
Implements IView
Private Sub UserForm_Initialize()
minHeight = Me.Height
minWidth = Me.Width
BindControlLayouts
End Sub
Private Sub BindControlLayouts()
Dim backgroundImageLayout As New ControlLayout
backgroundImageLayout.Bind Me, BackgroundImage, AnchorAll
Dim closeButtonLayout As New ControlLayout
closeButtonLayout.Bind Me, CloseButton, BottomAnchor + RightAnchor
Dim itemsListLayout As New ControlLayout
itemsListLayout.Bind Me, ItemsList, AnchorAll
Dim addButtonLayout As New ControlLayout
addButtonLayout.Bind Me, AddButton, RightAnchor
Dim editButtonLayout As New ControlLayout
editButtonLayout.Bind Me, EditButton, RightAnchor
Dim showDetailsButtonLayout As New ControlLayout
showDetailsButtonLayout.Bind Me, ShowDetailsButton, RightAnchor
Dim deleteButtonLayout As New ControlLayout
deleteButtonLayout.Bind Me, DeleteButton, RightAnchor
layoutBindings.Add closeButtonLayout, _
bac
Solution
There's usually very little I can say about your code, but maybe if I just start going down line by line I'll find something. I'm hoping someone else who knows more about mvp and dependency-injection come along as well.
Simple Form
Ok, so I only touched on the "Simple Form" (and just a bit on resources), but I suspect I could only give more of the same kind of advice. So, I'll leave the rest of the code for someone else.
Simple Form
- I don't see Types uses very often in vba. Normally when I do, what is really needed is a class. I don't think that's the case here, but if
ViewModelchanges down the road, it's not hard to switch.
- I think
Selectionwould be less ambiguous asSelectedRow.
- Ummmm........
vm..... I know VB being case insensitive sucks, but couldn't you have at least dsmvwld it? I bet you were avoiding (gasp) hungarian notation. Given the choice betweenvmandvwModel, I'll take the dsmvwld hngNotation.
- I would probably use a couple of constants in
UserForm_InitializeoverMe.HeightandMe.Width. It's too easy to change it accidentally in design mode.
BindControlLayoutlooks good. I like the use of bitwise enumeration.
UserForm_ResizeNeeds to have an error handler. Anytime you turn screen updating off, you'll need an error handler. Just in case.
- Okay, now I see why you went with
vmandSelection. The user form has propertiesModelandSelectedItem. Naming is hard in vb. You could still useSelectedItemfor the ViewModel Type though. I don't think it would cause any confusion.
- The localization is really frickin' cool. Seriously. That's cool. But, you've now bound the code to this specific workbook. What if you want to reuse this somewhere else? You're this far down the rabbit hole, why not create an interface that implements a dicitonary? I'm pretty sure I remember reading somewhere that you can leverage .Net's dictionary if the framework is installed on your users machines. (Hint: The .Net framework is installed on practically every windows machine.)
- All of the Click events have to check
If vm.Callback.CallbackOwner Is Nothing. There's some duplication here, but I don't feel it's a big deal. You might consider creating a boolean function, but that might be overkill. (cough-cough it is cough-cough)
Ok, so I only touched on the "Simple Form" (and just a bit on resources), but I suspect I could only give more of the same kind of advice. So, I'll leave the rest of the code for someone else.
Context
StackExchange Code Review Q#58348, answer score: 7
Revisions (0)
No revisions yet.