4
\$\begingroup\$

I work on a .NET application that very loosely follows an n-tier kind of architecture (the business objects (not logic) and data access are split out). We are now looking to start refactoring the code into an MVP architecture. Based on some previous work done by someone else, I just refactored a screen into what I hope is MVP.

The form code:

Imports System.Windows.Forms
Imports ExceptionLibrary.UIExceptionCatcher
Imports BusinessEntity.OE
Imports OurClient.MainModule.Constants

<System.ComponentModel.ToolboxItem(False)> _
Partial Public Class AutoService
    Inherits UILibrary.CustomViewControl
    Implements IAutoService

    Private _objAutoServiceBE As New AutoServiceBE
    Private _isEditMode As Boolean
    Private _intCurrentUserId As Integer
    Private _intNewAddedAutoServiceId As Integer
    Private _intAutoServiceID As Integer

    Public Event Close(ByVal sender As Object, ByVal e As EventArgs) Implements IAutoService.Close
    Public Event Save(ByVal sender As Object, ByVal e As EventArgs) Implements IAutoService.Save
    Public Event FormLoad(ByVal e As EventArgs) Implements IAutoService.ViewLoad

    Public Property AutoService As BusinessEntity.OE.AutoServiceBE Implements IAutoService.AutoService
        Get
            Return _objAutoServiceBE
        End Get
        Set(value As BusinessEntity.OE.AutoServiceBE)
            _objAutoServiceBE = value
        End Set
    End Property

    Public Property AutoServiceCode As String Implements IAutoService.AutoServiceCode
        Get
            Return txtCode.Text
        End Get
        Set(value As String)
            txtCode.Text = value
        End Set
    End Property

    Public Property AutoServiceName As String Implements IAutoService.AutoServiceName
        Get
            Return txtName.Text
        End Get
        Set(value As String)
            txtName.Text = value
        End Set
    End Property

    Public Property AutoServiceID() As Integer Implements IAutoService.AutoServiceID
        Get
            Return _intAutoServiceID
        End Get
        Set(ByVal value As Integer)
            _intAutoServiceID = value
        End Set
    End Property

    Public Property AutoServiceTypeList As IList(Of BusinessEntity.OE.AutoServiceBE) Implements IAutoService.AutoServiceTypeList
        Get
            Return cboAutoServiceType.DataSource
        End Get
        Set(value As IList(Of BusinessEntity.OE.AutoServiceBE))
            With cboAutoServiceType
                .DisplayMember = "AutoServiceTypeName"
                .ValueMember = "AutoServiceTypeID"
                .DataSource = value
                .SelectedItem = Me.SelectedAutoServiceType
            End With
        End Set
    End Property

    Public Property CursorType As Cursor Implements IAutoService.CursorType
        Get
            Return Me.Cursor
        End Get
        Set(value As Cursor)
            Me.Cursor = value
        End Set
    End Property

    Public Property ViewDisplayModeType As UILibrary.CustomViewControl.ViewDisplayMode Implements IAutoService.ViewDisplayMode
        Get
            Return Me.DisplayMode
        End Get
        Set(value As UILibrary.CustomViewControl.ViewDisplayMode)
            Me.DisplayMode = value
        End Set
    End Property

    Public Property EditMode() As String Implements IAutoService.EditMode
        Get
            Return _isEditMode
        End Get
        Set(ByVal value As String)
            _isEditMode = value
        End Set
    End Property

    Public Property NewAddedAutoServiceId() As Integer Implements IAutoService.NewAddedAutoServiceId
        Get
            Return _intNewAddedAutoServiceId
        End Get
        Set(ByVal value As Integer)
            _intNewAddedAutoServiceId = value
        End Set
    End Property

    Public Property IsNameSelected As Boolean Implements IAutoService.IsNameSelected
        Get
            Return txtName.Focused
        End Get
        Set(value As Boolean)
            txtName.Focus()
            txtName.SelectAll()
        End Set
    End Property

    Public WriteOnly Property IsNameFailed As Boolean Implements IAutoService.IsNameFailed
        Set(value As Boolean)
            ShowFailFrame(txtName)
        End Set
    End Property

    Public Property IsCodeSelected As Boolean Implements IAutoService.IsCodeSelected
        Get
            Return txtCode.Focused
        End Get
        Set(value As Boolean)
            txtCode.Focus()
            txtCode.SelectAll()
        End Set
    End Property

    Public WriteOnly Property IsCodeFailed As Boolean Implements IAutoService.IsCodeFailed
        Set(value As Boolean)
            ShowFailFrame(txtCode)
        End Set
    End Property

    Public Property PvcCloseResult As DialogResult Implements IAutoService.PvcCloseResult
        Get
            Return CloseResult
        End Get
        Set(value As DialogResult)
            CloseResult = value
        End Set
    End Property

    Public Property PvcIsViewDirty As Boolean Implements IAutoService.PvcIsViewDirty
        Get
            Return IsViewDirty
        End Get
        Set(value As Boolean)
            IsViewDirty = value
        End Set
    End Property

    Public ReadOnly Property PvcValidateRequiredFields As Boolean Implements IAutoService.PvcValidateRequiredFields
        Get
            Return ValidateRequiredFields()
        End Get
    End Property

    Public WriteOnly Property PvcSaveCallback As [Delegate] Implements IAutoService.PvcSaveCallback
        Set(value As [Delegate])
            CallbackFunctionForSave = value
        End Set
    End Property

    Public Property UserID As Integer Implements IAutoService.UserID
        Get
            Return _intCurrentUserId
        End Get
        Set(value As Integer)
            _intCurrentUserId = value
        End Set
    End Property

    Public Property SelectedAutoServiceType As BusinessEntity.OE.AutoServiceBE Implements IAutoService.SelectedAutoServiceType
        Get
            Return cboAutoServiceType.SelectedItem
        End Get
        Set(value As BusinessEntity.OE.AutoServiceBE)
            cboAutoServiceType.SelectedItem = value
        End Set
    End Property

    Public Sub New()
        InitializeComponent()
    End Sub

    Protected Overrides Sub OnLoad(ByVal e As EventArgs)
        Try
            _presenter.OnViewReady()
            MyBase.OnLoad(e)

            Cursor = Cursors.WaitCursor
            Hide()
            TitleBarText = "AutoService"
            DisplayStyle = ViewDisplayStyle.DialogBox
            ViewAcceptButton = btnSave
            ViewCancelButton = btnClose

            Me.UserID = SQLDataServices.GlobalUserID
            Me.AutoService.CreateUserID = Me.UserID

            AddHandlers()

            InitializeDirtyTracking()
            InitializeValidationTracking()

            RaiseEvent FormLoad(e)

            cboAutoServiceType.Select()
        Catch ex As Exception
            HandleException(ex, Me.ToString, "OnLoad", True, True)
            Me.Enabled = False
        Finally
            Me.Show()
            Cursor = Cursors.Default
        End Try
    End Sub

    Private Sub OnClick_btnSave(ByVal sender As System.Object, ByVal e As System.EventArgs)
        RaiseEvent Save(sender, e)
    End Sub

    Private Sub OnClick_btnClose(ByVal sender As System.Object, ByVal e As System.EventArgs)
        CloseResult = DialogResult.Cancel
        RaiseEvent Close(sender, e)
        Me.ParentForm.Close()
    End Sub

    Private Sub AddHandlers()
        AddHandler btnSave.Click, AddressOf OnClick_btnSave
        AddHandler btnClose.Click, AddressOf OnClick_btnClose
    End Sub

    Private Sub InitializeDirtyTracking()
        IsViewDirty = False
        AddToDirtyTracker(cboAutoServiceType)
        AddToDirtyTracker(txtName)
        AddToDirtyTracker(txtCode)
    End Sub

    Private Sub InitializeValidationTracking()
        AddToValidationTracker(cboAutoServiceType)
        AddToValidationTracker(txtName)
        AddToValidationTracker(txtCode)
    End Sub

    Private Sub SaveToDB() Implements IAutoService.SaveToDB
        RaiseEvent Save(Me, Nothing)
    End Sub

    Public Sub ShowView() Implements IAutoService.Show
        ViewServices.ShowSmartPart(Me, WorkspaceNames.MDIWorkspace, True)
    End Sub

    Public Sub CloseView() Implements IAutoService.CloseView
        RaiseEvent Close(Me, Nothing)
        Me.ParentForm.Close()
    End Sub
End Class

The presenter code:

Imports System.Windows.Forms
Imports CommonUILibrary.UserMessage
Imports ExceptionLibrary.UIExceptionCatcher
Imports OurClient.Infrastructure.Interface

Partial Public Class AutoServicePresenter
    Inherits Presenter(Of IAutoService)

    Private _dataService As IAutoServiceDataService

    Public Overrides Sub OnViewReady()
        MyBase.OnViewReady()
    End Sub

    Public Sub SubscribeToEvents()
        AddHandler View.Close, AddressOf OnClose
        AddHandler View.Save, AddressOf OnSave
        AddHandler View.ViewLoad, AddressOf OnViewLoad
    End Sub

    Public Sub OnClose(ByVal sender As Object, ByVal e As EventArgs)
    End Sub

    Public Sub OnSave(ByVal sender As Object, ByVal e As EventArgs)
        View.CursorType = Cursors.WaitCursor

        Try
            If SaveToDatabase() Then
                View.PvcCloseResult = DialogResult.OK
                View.CloseView()
            End If
        Catch ex As Exception
            HandleException(ex, Me.ToString, "OnClick_btnSave", True, True)
        Finally
            View.CursorType = Cursors.Default
        End Try
    End Sub

    Public Sub OnViewLoad(e As EventArgs)
        View.AutoServiceTypeList = _dataService.GetAutoServiceTypes()

        If View.ViewDisplayMode = ViewDisplayMode.Edit Then
            View.AutoService = _dataService.GetAutoService(View.AutoServiceID)

            View.AutoServiceName = View.AutoService.AutoServiceName
            View.AutoServiceCode = View.AutoService.AutoServiceCode
            View.SelectedAutoServiceType = _dataService.GetAutoServiceType(View.AutoServiceTypeList, View.AutoService.AutoServiceTypeID)
        Else
            View.AutoServiceName = ""
            View.AutoServiceCode = ""
            View.SelectedAutoServiceType = _dataService.GetAutoServiceType(View.AutoServiceTypeList, View.AutoService.AutoServiceTypeID)
        End If
    End Sub

    Public Sub UpdateGridView()
        If WorkItem.SmartParts.Contains("AutoServiceNames") Then
            Dim objSetupAutoServiceNames As SetupAutoServiceNames = WorkItem.SmartParts.Get(Of SetupAutoServiceNames)("AutoServiceNames")
            objSetupAutoServiceNames.OnPopulateGrid()
        End If
    End Sub

    Private Function SaveToDatabase() As Boolean
        Dim intResult As Integer = -1

        If ValidateView() Then
            SetViewDataToBE()

            If View.ViewDisplayMode = ViewDisplayMode.Add Then      'new autoservice
                intResult = _dataService.AddAutoService(View.AutoService)

                If intResult > 0 Then
                    UpdateGridView()
                    AppDataCache.UpdateCache(AppDataCache.SubjectAreaName.OrderEntry, True)
                    View.PvcIsViewDirty = False
                    ShowInformation("Auto Service added successfully.")
                ElseIf intResult = -1 Then
                    View.IsNameFailed = True
                    ShowWarning("An Auto Service with the same name already exists.")
                    View.IsNameSelected = True
                ElseIf intResult = -2 Then
                    View.IsCodeFailed = True
                    ShowWarning("An Auto Service with the same code already exists.")
                    View.IsCodeSelected = True
                ElseIf intResult = -3 Then
                    View.IsNameFailed = True
                    ShowInformation("Please call the help desk to reuse the Auto Service Name for this Auto Service Type.")
                    View.IsNameSelected = True
                ElseIf intResult = -4 Then
                    View.IsCodeFailed = True
                    ShowInformation("Please call the help desk to reuse the Auto Service Code for this Auto Service Type.")
                    View.IsCodeSelected = True
                Else
                    ShowWarning("Failed to add the Auto Service.")
                End If

            Else : View.ViewDisplayMode = ViewDisplayMode.Edit      'autoservice in edit mode
                intResult = _dataService.EditAutoService(View.AutoService)

                If intResult > 0 Then
                    UpdateGridView()
                    AppDataCache.UpdateCache(AppDataCache.SubjectAreaName.OrderEntry, True)
                    View.PvcIsViewDirty = False
                    ShowInformation("Auto Service updated successfully.")
                ElseIf intResult = -1 Then
                    View.IsNameFailed = True
                    ShowWarning("An Auto Service with the same name already exists.")
                    View.IsNameSelected = True
                ElseIf intResult = -2 Then
                    View.IsCodeFailed = True
                    ShowWarning("An Auto Service with the same code already exists.")
                    View.IsCodeSelected = True
                ElseIf intResult = -3 Then
                    View.IsNameFailed = True
                    ShowWarning("Please call the help desk to reuse the Auto Service Name for this Auto Service Type.")
                    View.IsNameSelected = True
                ElseIf intResult = -4 Then
                    View.IsCodeFailed = True
                    ShowWarning("Please call the help desk to reuse the Auto Service Code for this Auto Service Type.")
                    View.IsCodeSelected = True
                Else
                    ShowWarning("Failed to update the Auto Service.")
                End If
            End If
        End If

        View.NewAddedAutoServiceId = intResult

        Return (intResult > 0)
    End Function

    Public Function ValidateView() As Boolean
        Dim blnReturn As Boolean = True

        If View.PvcValidateRequiredFields Then
            'All' Name should not be created
            If (View.AutoServiceName.Trim.ToLower = "All".ToLower) Then
                View.IsNameFailed = True
                ShowWarning("Auto Service Name cannot be" + " '" + View.AutoServiceName + "' ")
                View.IsNameSelected = True
                blnReturn = False
            End If
        Else
            blnReturn = False
        End If

        Return blnReturn
    End Function

    Private Sub SetViewDataToBE()

        'assigning control values to the object
        With View.AutoService
            .AutoServiceName = View.AutoServiceName
            .AutoServiceCode = View.AutoServiceCode

            If View.SelectedAutoServiceType IsNot Nothing Then
                .AutoServiceTypeID = View.SelectedAutoServiceType.AutoServiceTypeID
            End If

            .AutoServiceID = View.AutoServiceID
            .CreateUserID = View.UserID
        End With
        If View.ViewDisplayMode = ViewDisplayMode.Add And View.SelectedAutoServiceType IsNot Nothing Then
            View.AutoServiceTypeList.Add(View.SelectedAutoServiceType)
        End If
    End Sub

    Public Sub New()
        MyBase.New()
        _dataService = New AutoServiceDataService
    End Sub
End Class

The DataService code:

Public Class AutoServiceDataService
    Implements IAutoServiceDataService

    Public Function AddAutoService(ByRef objAutoServiceBE As BusinessEntity.OE.AutoServiceBE) As Integer Implements IAutoServiceDataService.AddAutoService
        Dim intReturn As Integer = 0

        intReturn = OurClientDAL.OEService.AutoServiceNameDAL.InsertAutoService(objAutoServiceBE)

        Return intReturn
    End Function

    Public Function EditAutoService(ByRef objAutoServiceBE As BusinessEntity.OE.AutoServiceBE) As Integer Implements IAutoServiceDataService.EditAutoService
        Dim intReturn As Integer = 0

        intReturn = OurClientDAL.OEService.AutoServiceNameDAL.UpdateAutoService(objAutoServiceBE)

        Return intReturn
    End Function

    Public Function GetAutoService(intAutoServiceID As Integer) As BusinessEntity.OE.AutoServiceBE Implements IAutoServiceDataService.GetAutoService
        Return OurClientDAL.OEService.AutoServiceNameDAL.GetAutoServiceByID(intAutoServiceID)
    End Function

    Public Function GetAutoServiceType(ByVal lstAutoService As List(Of BusinessEntity.OE.AutoServiceBE), _
                                       ByVal intAutoServiceTypeID As Integer) As BusinessEntity.OE.AutoServiceBE Implements IAutoServiceDataService.GetAutoServiceType
        Dim objReturn As New BusinessEntity.OE.AutoServiceBE

        For Each objAutoService As BusinessEntity.OE.AutoServiceBE In lstAutoService
            If objAutoService.AutoServiceTypeID = intAutoServiceTypeID Then
                objReturn = objAutoService
                Exit For
            End If
        Next

        Return objReturn
    End Function

    Public Function GetAutoServiceTypes() As IList(Of BusinessEntity.OE.AutoServiceBE) Implements IAutoServiceDataService.GetAutoServiceTypes
        Return OurClientDAL.OEService.AutoServiceNameDAL.GetAutoServiceType()
    End Function
End Class

I initially asked a similar question over at Stack Overflow, though it garnered little attention. I have reprhased it in the context of a code review so I can make sure that I am doing this "right."

\$\endgroup\$
3
\$\begingroup\$
Public ReadOnly Property PvcValidateRequiredFields As Boolean Implements IAutoService.PvcValidateRequiredFields
    Get
        Return ValidateRequiredFields()
    End Get
End Property

I looked through the code and this looked really odd to me, so I went looking for the method ValidateRequiredFields() and could not find it in the code that you have provided.

instead of creating a property that returns that value of a method/function you should just allow the method/function to be called. That method should return a boolean (true/false) value. Are the required fields there or not? that should be available to the caller.

Why did you leave this code out of the question? I don't know.

This doesn't seem to be the complete code set

\$\endgroup\$
  • \$\begingroup\$ It has been a while, but it was done this way becuase PVC is a view control, of sorts. It contains many utility methods, and all of our forms inherit from it (any that don't get refactored when we touch them). This screen in particular needed to use those same methods (like changing the color of a textbox to indicate it is a required field, and then to validate that it has data). If that is the only thing you could find wrong with the code, I assume all is well. \$\endgroup\$ – Dave Johnson Dec 14 '15 at 17:20
  • 1
    \$\begingroup\$ I will take a closer look at it later, but it looks like everything was pretty good. others are now looking at this as well so anything askew should get fleshed out in the next day or so \$\endgroup\$ – Malachi Dec 14 '15 at 17:21
  • \$\begingroup\$ The trick is that this screen, being MVP, inherits the CustomViewControl, as do all of the MVP screens we have. But, it also needed access to our "PVC", so the CustomViewControl ultimately inheriets (or maybe Implements) the PVC, and then it is accessed in this way. \$\endgroup\$ – Dave Johnson Dec 14 '15 at 17:22
  • \$\begingroup\$ then you should inherit its method as well, right? \$\endgroup\$ – Malachi Dec 15 '15 at 0:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.