HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Down the rabbit hole with MVP

Submitted by: @import:stackexchange-codereview··
0
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 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

  • 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 ViewModel changes down the road, it's not hard to switch.



  • I think Selection would be less ambiguous as SelectedRow.



  • 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 between vm and vwModel, I'll take the dsmvwld hngNotation.



  • I would probably use a couple of constants in UserForm_Initialize over Me.Height and Me.Width. It's too easy to change it accidentally in design mode.



  • BindControlLayout looks good. I like the use of bitwise enumeration.



  • UserForm_Resize Needs 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 vm and Selection. The user form has properties Model and SelectedItem. Naming is hard in vb. You could still use SelectedItem for 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.