Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

Still pursuing the white rabbitStill pursuing the white rabbit, I had an IPresenter interface implementation featuring this method:

Still pursuing the white rabbit, I had an IPresenter interface implementation featuring this method:

Still pursuing the white rabbit, I had an IPresenter interface implementation featuring this method:

Source Link
Mathieu Guindon
  • 75.1k
  • 18
  • 191
  • 463

Decoupling Presenter from "child" Repository

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 RefreshCommand

End Sub

The 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 Explicit

Private Type tPresenter
    MasterId As Long
    Repository As IRepository
    DetailsPresenter As IPresenter
    View As IView
End Type

Private this As tPresenter
Implements IPresenter

Private 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 = result
    
End Function

Public Property Get Repository() As IRepository
    Set Repository = this.Repository
End Property

Public Property Set Repository(ByVal value As IRepository)
    Set this.Repository = value
End Property

Private Function IPresenter_CanExecuteCommand(ByVal commandId As CommandType) As Boolean
    If commandId = FindCommand Then IPresenter_CanExecuteCommand = True
End Function

Private Property Set IPresenter_DetailsPresenter(ByVal value As IPresenter)
'not implemented
End Property

Private Property Get IPresenter_DetailsPresenter() As IPresenter
'not implemented
End Property

Private Function IPresenter_ExecuteCommand(ByVal commandId As CommandType) As Variant
    
    Select Case commandId
        
        Case CommandType.FindCommand
            IPresenter_ExecuteCommand = ExecuteFindCommand
            
        Case Else
            'not implemented
            
    End Select
    
End Function

Private 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 = orderId
    
End Function

Private Property Let IPresenter_MasterId(ByVal value As Long)
    this.MasterId = value
End Property

Private Property Get IPresenter_MasterId() As Long
    IPresenter_MasterId = this.MasterId
End Property

Private Property Set IPresenter_Repository(ByVal value As IRepository)
    Set Repository = value
End Property

Private Property Get IPresenter_Repository() As IRepository
    Set IPresenter_Repository = Repository
End Property

Private Sub IPresenter_Show()
'not implemented
End Sub

Private Property Set IPresenter_View(ByVal value As IView)
'not implemented
End Property

Private Property Get IPresenter_View() As IView
'not implemented
End Property

For 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 RefreshCommand

End Sub

Here's the OrderHeaderRepository implementation, which partly explains the holes in the presenter implementation:

Option Explicit
Private cmd As New SqlCommand
Private Const selectString As String = "SELECT Id, Number, OrderDate, OrderTypeId, SeasonId FROM Planning.OrderHeaders"
Implements IRepository

Private Sub IRepository_Add(ByVal value As SqlResultRow)
'not implemented
End Sub

Private Function IRepository_Count() As Long
    IRepository_Count = RepositoryImpl.Count("Planning.OrderHeaders")
End Function

Private 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 Function

Private Function IRepository_GetAll() As SqlResult
'not implemented. too many results to efficiently process in a SqlResult.
End Function

Private Function IRepository_GetById(ByVal id As Long) As SqlResultRow
    Set IRepository_GetById = RepositoryImpl.GetById(selectString, id)
End Function

Private Function IRepository_NewItem(ByVal Model As SqlResult, ByVal values As Scripting.IDictionary) As SqlResultRow
    Set IRepository_NewItem = RepositoryImpl.NewItem(Model, values)
End Function

Private Sub IRepository_Remove(ByVal id As Long)
'not implemented
End Sub

Private Function IRepository_Search(ByVal terms As SqlResultRow) As SqlResult
'not implemented. possibly too many results to efficiently process with a SqlResult.
End Function

Private Sub IRepository_Update(ByVal id As Long, ByVal value As SqlResultRow)
'not implemented
End Sub

A 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?