Bad smell in code - Middle Man

If you ask anyone in the team what are the major components in our code base, more or less you will get a list like the following:

UI
ProcessController
BusinessProcessController
BusinessObjects
DataAccess
DataEntity


the architecture of the system was designed as following:

UI : keeps all windows forms and User controls
ProcessController: provides user process logic and interact with WCF service via a Proxy component
BusinessProcessController : a facade layer around BusinessObject providing WCF service
BusinessObjects : where the business Logic is
DataAccess: Database persistent layer ( getter and setter between BusinessObejct and database engine)
DataEntity: the carrier of business data among different layers.


Since our system is such complex system, one would imagine ProcessController and the BusinessObjects are the busiest components. They may have more line of code, they may have most functions/subs, they may have highest line code each method. After all, they are the brains of the system. UI is just some user interface, BusinessProcessController is just a facade layer around BO layer, DataAccess contains some brainless methods save and retrieve data back and from database. I wouldn't blame you. as I would think the same way.

However, if you look at the code metrics, you would see different picture:

Project Total #LOC Total # members Average LOC per member
UI 396,770 48,335 8
DataAccess 6,7313 3,323 20
DataEntity 65,306 31,230 2
BusinessObjects 40,291 3,331 12
BusinessProcessController 31,555 3,227 11
ProcessController 30,380 2,863 9
total 60,1235 ( 76% of overall LOC)


It looks like the busiest components are UI and DataAccess. BusinessProcessController is not as thin as we thought to be. why is that so, what do BusinessObjects and ProcessController do? Let's take a closer look at these 2 components.

well, besides whole bunch ( hundreds of them) of empty methods like the following:


Public Function GetAssignedItemsForGroup(ByVal GroupId As Long) As Microsoft.VisualBasic.Collection
Return Nothing
End Function

Public Sub ReassignActivity(ByVal coll As Collection)
End Sub

Public Sub ShowFeeSetup() Implements IPOSMain.ShowFeeSetup
End Sub

Public Function GetSuspendedItemsForGroup(ByVal GroupId As Long) As Microsoft.VisualBasic.Collection
Return Nothing
End Function

there are a bunch ( hundreds of them) of middle men like the following:

Public Sub UpdateFeeRuleDetails(ByVal objFeeSetUpBE As Server.POSDataEntity.FeeSetup.FeeSetupBE) Implements [Interface].FeeSetup.IFeeSetupDetails.UpdateFeeRuleDetails
txnAdminBpc.UpdateFeeRuleDetails(objFeeSetUpBE)
End Sub

Public Sub AddTitleDetailsForNonTitledVehicle(ByVal title As TitleBE, ByVal staffId As String, ByVal ownershipDocId As Long, ByVal txnId As Long)
mTitleDAO.AddTitleDetailsForNonTitledVehicle(title, staffId, ownershipDocId, txnId)
End Sub
Public Sub AddExistingTitleDetailsForInhouseCorrection(ByVal title As TitleBE, ByVal txnId As Long, ByVal createdBy As String, ByVal ownershipDocId As Long, ByVal isCorrected As Boolean)
mTitleDAO.AddExistingTitleDetailsForInhouseCorrection(title, txnId, createdBy, ownershipDocId, isCorrected)
End Sub

Public Sub AddTitleDetailsByTitleId(ByVal title As TitleBE, ByVal ownershipDocId As Long, ByVal transactionId As Long, ByVal userId As String)
mTitleDAO.AddTitleDetailsByTitleId(title, ownershipDocId, transactionId, userId)
End Sub

Public Sub AddExistingTitleDetailsByTitleId(ByVal title As TitleBE, ByVal ownershipDocId As Long)
mTitleDAO.AddExistingTitleDetailsByTitleId(title, ownershipDocId)
End Sub


Because they are middle men here, we end up let the UI , BusinessProcessController and DataAccess components do the heavy lifting. That's why you see the LOC in UI and BusinessProcessController so high.

However, I am not suggesting to get rid of BusinessObjects or ProcessController. They should be there as they were originally designed. What I am suggesting is do not let them be the middle men. Give them some work to do to justify their existence. If these method is empty, we should remove them. If these methods just be the middle man, ask them to do some more. Move the logic in UI, DataAccess and BusinessProcessController to where it belong to. When you do that, you will find the total line of code in the system been reduced. why? because when the logic been moved to ProcessController and BusinessObjects, you will eliminate lots of repeated logic in UI, BusinessProcessController and many other places . You will enjoy much more when working on the codebase. Less defects, less places to change when fix defects.