Still pursuing the white rabbit, I had an IPresenter interface implementation featuring this method:Private Sub ExecuteAddCommand()    Dim orderNumber As String    If Not RequestUserInput(prompt:=GetResourceString(PromptExcludedOrderNumberMessageText), _                            title:=GetResourceString(AddPromptTitle), _                            outResult:=orderNumber, _                            default:=0) _    Then        Exit Sub    End If    'tight coupling...    Dim orderRepo As IRepository    Set orderRepo = New OrderHeaderRepository    Dim values As New Dictionary    values.Add number, orderNumber    Dim orderModel As New SqlResult    orderModel.AddFieldName Number    Dim order As SqlResultRow    Set order = orderRepo.NewItem(orderModel, values)    Dim orderId As Long    orderId = orderRepo.FindId(order)    If orderId = 0 Then        MsgBox StringFormat(GetResourceString(InvalidOrderNumberMessageText), orderNumber), _               vbExclamation, _               GetResourceString(InvalidOrderNumberTitle)        Exit Sub    End If    Dim reason As String    If Not RequestUserInput(prompt:=GetResourceString(AddExcludedOrderMessageText), _                            title:=GetResourceString(AddPromptTitle), _                            outResult:=reason, _                            default:=GetResourceString(DefaultExcludedOrderReason)) _    Then        Exit Sub    End If    Repository.Add NewExcludedOrder(orderHeaderId:=orderId, reason:=reason)    IPresenter_ExecuteCommand RefreshCommandEnd SubThe problem is that, out of 12 implementations so far, it was the only one with such tight coupling - for every single other case, it made sense to implement a child presenter with its own repository.The tightly coupled code works, but can't be tested offline, which was the entire point/purpose of going down the rabbit hole with implementing MVP and Repository patterns in VBA. I had to do something about it.So I added a FindCommand to the CommandType enum, and implemented an OrderHeaderPresenter:Option ExplicitPrivate Type tPresenter    MasterId As Long    Repository As IRepository    DetailsPresenter As IPresenter    View As IViewEnd TypePrivate this As tPresenterImplements IPresenterPrivate Function NewOrderHeader(Optional ByVal id As Long = 0, Optional ByVal number As String = vbNullString) As SqlResultRow    Dim result As SqlResultRow    Dim values As New Dictionary    values.Add id, id    values.Add number, number    Dim orderModel As New SqlResult    orderModel.AddFieldName id    orderModel.AddFieldName number    Set result = Repository.NewItem(orderModel, values)    Set NewOrderHeader = resultEnd FunctionPublic Property Get Repository() As IRepository    Set Repository = this.RepositoryEnd PropertyPublic Property Set Repository(ByVal value As IRepository)    Set this.Repository = valueEnd PropertyPrivate Function IPresenter_CanExecuteCommand(ByVal commandId As CommandType) As Boolean    If commandId = FindCommand Then IPresenter_CanExecuteCommand = TrueEnd FunctionPrivate Property Set IPresenter_DetailsPresenter(ByVal value As IPresenter)'not implementedEnd PropertyPrivate Property Get IPresenter_DetailsPresenter() As IPresenter'not implementedEnd PropertyPrivate Function IPresenter_ExecuteCommand(ByVal commandId As CommandType) As Variant    Select Case commandId        Case CommandType.FindCommand            IPresenter_ExecuteCommand = ExecuteFindCommand        Case Else            'not implemented    End SelectEnd FunctionPrivate Function ExecuteFindCommand() As Long    Dim orderNumber As String    If Not RequestUserInput(prompt:=GetResourceString(PromptExcludedOrderNumberMessageText), _                            title:=GetResourceString(AddPromptTitle), _                            outResult:=orderNumber, _                            default:=0) _    Then        Exit Function    End If    Dim order As SqlResultRow    Set order = NewOrderHeader(number:=orderNumber)    Dim orderId As Long    orderId = Repository.FindId(order)    If orderId = 0 Then        MsgBox StringFormat(GetResourceString(InvalidOrderNumberMessageText), orderNumber), _               vbExclamation, _               GetResourceString(InvalidOrderNumberTitle)        Exit Function    End If    ExecuteFindCommand = orderIdEnd FunctionPrivate Property Let IPresenter_MasterId(ByVal value As Long)    this.MasterId = valueEnd PropertyPrivate Property Get IPresenter_MasterId() As Long    IPresenter_MasterId = this.MasterIdEnd PropertyPrivate Property Set IPresenter_Repository(ByVal value As IRepository)    Set Repository = valueEnd PropertyPrivate Property Get IPresenter_Repository() As IRepository    Set IPresenter_Repository = RepositoryEnd PropertyPrivate Sub IPresenter_Show()'not implementedEnd SubPrivate Property Set IPresenter_View(ByVal value As IView)'not implementedEnd PropertyPrivate Property Get IPresenter_View() As IView'not implementedEnd PropertyFor this to work, I had to change IPresenter.ExecuteCommand from a Sub to a Function returning a Variant, so that commands like Find and Search can return an object or a value.And that allowed me to refactor my coupled presenter method to this:Private Sub ExecuteAddCommand()    Dim orderId As Long    orderId = this.DetailsPresenter.ExecuteCommand(FindCommand)    If orderId = 0 Then Exit Sub    Dim reason As String    If Not RequestUserInput(prompt:=GetResourceString(AddExcludedOrderMessageText), _                            title:=GetResourceString(AddPromptTitle), _                            outResult:=reason, _                            default:=GetResourceString(DefaultExcludedOrderReason)) _    Then        Exit Sub    End If    Repository.Add NewExcludedOrder(orderHeaderId:=orderId, reason:=reason)    IPresenter_ExecuteCommand RefreshCommandEnd SubHere's the OrderHeaderRepository implementation, which partly explains the holes in the presenter implementation:Option ExplicitPrivate cmd As New SqlCommandPrivate Const selectString As String = SELECT Id, Number, OrderDate, OrderTypeId, SeasonId FROM Planning.OrderHeadersImplements IRepositoryPrivate Sub IRepository_Add(ByVal value As SqlResultRow)'not implementedEnd SubPrivate Function IRepository_Count() As Long    IRepository_Count = RepositoryImpl.Count(Planning.OrderHeaders)End FunctionPrivate Function IRepository_FindId(ByVal naturalKey As SqlResultRow) As Long    Dim sql As String    sql = SELECT Id FROM Planning.OrderHeaders WHERE Number = ?;    Dim result As Long    'todo: find out why query won't return a value with a string parameter.    '      underlying db table column is a varchar, doesn't make sense to CLng() here.    result = cmd.QuickSelectSingleValue(sql, CLng(naturalKey(number)))    IRepository_FindId = IIf(IsEmpty(result), 0, result)End FunctionPrivate Function IRepository_GetAll() As SqlResult'not implemented. too many results to efficiently process in a SqlResult.End FunctionPrivate Function IRepository_GetById(ByVal id As Long) As SqlResultRow    Set IRepository_GetById = RepositoryImpl.GetById(selectString, id)End FunctionPrivate Function IRepository_NewItem(ByVal Model As SqlResult, ByVal values As Scripting.IDictionary) As SqlResultRow    Set IRepository_NewItem = RepositoryImpl.NewItem(Model, values)End FunctionPrivate Sub IRepository_Remove(ByVal id As Long)'not implementedEnd SubPrivate Function IRepository_Search(ByVal terms As SqlResultRow) As SqlResult'not implemented. possibly too many results to efficiently process with a SqlResult.End FunctionPrivate Sub IRepository_Update(ByVal id As Long, ByVal value As SqlResultRow)'not implementedEnd SubA few things annoy me slightly:Most of both IPresenter and IRepository interfaces are left unimplemented for the OrderHeader model - the presenter is only exposing a Repository property... which somewhat smells.The OrderHeaderPresenter, while physically decoupled from the ExcludedOrdersPresenter, is still functionally coupled with it - the FindCommand is prompting the user for an order number to be excluded, which is what needs to happen, but the OrderHeaderPresenter shouldn't be using resources meant to be used in another implementation. Or should I just change that string to read something more generic, like Please enter an order number:?Anything else sticks out? Any recommendations?
Decoupling Presenter from child Repository
object oriented;design patterns;vba
Let's talk a little about your code the way it is right now. This happens a lot.Private Property Set IPresenter_DetailsPresenter(ByVal value As IPresenter)'not implementedEnd PropertyPrivate Property Get IPresenter_DetailsPresenter() As IPresenter'not implementedEnd PropertyI like that you took the time to comment that it's not implemented, but anyone calling OrderHeaderPresenter.DetailsPresenter will have to dive into the source code to find out why it's not doing anything. Worse, it might take the unwary dev a long time to realize that it isn't doing anything. All of these should raise FeatureNotImplementedErrors.Putting that aside, you're right. The simple fact that all of these are not implemented does smell, but we'll get back to that.Private Function IPresenter_ExecuteCommand(ByVal commandId As CommandType) As Variant   Select Case commandId       Case CommandType.FindCommand           IPresenter_ExecuteCommand = ExecuteFindCommand       Case Else            'not implemented   End SelectEnd FunctionThe Else case here makes me think that OrderHeaderPresenter isn't really a IPresenter. It only implements one of the command types and you added that command type specifically for this implementation. At the very least, don't let it silently fail. Again, raise an error here.Private Function ExecuteFindCommand() As Long    Dim orderNumber As String    If Not RequestUserInput(prompt:=GetResourceString(PromptExcludedOrderNumberMessageText), _Your execute find command excludes the order number it's given? How is that a find command. Find is for positive hits, not negatives. Granted, I don't have a better name for you, but think on it.The order ID is added to some ExcludedOrders table.Okay, that's unclear from solely reading the code. Add a comment here.Private Sub IPresenter_Show()'not implementedEnd SubIf that doesn't convince you that this isn't a IPresenter I don't know what will. It's a presenter that doesn't present anything.A lot of the same with IRepository methods not being implemented.So, that's an awful lot of Raise errors and This isn't really a IPresenter, but not a lot of advice on how to fix it. In my mind, there aren't a lot of options.You could simply raise the custom errors I've mentioned and be done with it. (Easiest, but not necessarily the right thing to do.)Create two new interfaces. I think creating a IRepositoryBase that holds the common contracts makes sense. OrderDetailsHeader would implement only IRepositoryBase and all of your other existing classes would need to be changed to implement both IRepositoryBase and IRepository. The second new interface would be for those presenter methods that aren't really presenter methods. (By far the hardest, but perhaps the most correct option.)Create a single new interface for this new class. It will duplicate some code from both IRepository and IPresenter. I would opt for option number three myself. I hope I've done enough to convince you that OrderDetailsPresenter is not really the same thing as an IPresenter. Making it it's own thing makes sense. This could also be considered the most correct thing to do because you really should never change an interface once it's been created and used. To quote CPearson:Once you have defined your interface class module, you must not change the interface. Do not add, change, or remove any procedures or properties. Doing so will break any class that implements the interface and all those classes will need to be modified. Once defined, the interface should be viewed as a contract with the outside world, because other objects depend on the form of the interface. If you modify an interface, any class or project that implements that interface will not compile and all the classes will need to be modified to reflect the changes in the interface. This is not good. In a large application, particularly one developed by more than one programmer, your interface may be used in places of which you are not aware. Changing the interface will break code in unexpected places. If you find the absolute need to change an interface, leave the original interface unchanged and create a new interface with the desired modifications. This way, new code can implement the new interface, but existing code will not be broken when using the original interface.Emphasis mine.