Monday, June 23, 2014

FW: bad smell code # 4 Long Parameter List

I took a look at our current code base; I found the top 4 longest parameter list members as following:
 
TROrignalTransferTitleMessageBE.New(
ByVal TRStreet As String, ByVal TRCity As String, ByVal TRState As String,
ByVal TRZipCode As String, ByVal TRZipCodeExt As String,
ByVal TRNamePartOne As String, ByVal TRNamePartTwo As String, ByVal TR_Handicap_Sticker As String, _
ByVal TR_DLN As String, ByVal TR_Passport_Indicator As String, ByVal TR_Number_Of_Plates As String, _
ByVal TR_Dealer_Number As String, ByVal TR_Orig_Registration_Total As Decimal, ByVal TR_Registration_Transaction_Code As String,
ByVal TR_Title_Transaction_Code As String, ByVal TR_Tab_Number As String, ByVal TR_Plate_Number As String,
ByVal TR_Prev_Plate_Number As String, ByVal TR_Number_Months As Short, ByVal TRExpirationDate As String,
ByVal TR_Replacement_Tab_Indicator As String, ByVal TRCountyCode As String, ByVal TR_Model_Year As Short,
ByVal TR_Make As String, ByVal TR_VIN As String, ByVal TR_Body_Style_Code As String,
ByVal TR_Weight As Long, ByVal TR_Fee_Field_Change_Indicator As String, ByVal TR_Weight_Ind As String,
ByVal TR_Instant_Title_Code As String, ByVal TR_MH_App_Type As String, ByVal TRDPC As String,
ByVal TR_Graphic_Plate_Indicator As String, ByVal TR_Manufacturer_Plate_Format As String, ByVal TR_Insurance_Indicator As String,
ByVal TR_Payment_Code As String, ByVal TRFlashStatusPlateNumber As String, ByVal TRRemovedRegistrationFlashStatusCode As String,
ByVal TRAddRegistrationFlashStatusCode As String, ByVal TR_Plate_Description As String, ByVal TROrgCode As String, ByVal FirstLienName As String, ByVal SecondLienName As String, ByVal firstLoanDate As String,
ByVal secondLoanDate As String, ByVal titleFees As Decimal, ByVal fullRightsIndicator As String, _
ByVal specialCodes As String, ByVal specialUseCode As String, ByVal taxExemptReason As String,
ByVal foreignTitleSateCode As String, ByVal titleNumber As String, ByVal vehicleColor As String,
ByVal salesPrice As Decimal, ByVal mileage As String, ByVal firstLienStreet As String,
ByVal firstLienCity As String, ByVal firstLienSate As String, ByVal firstLienZipCode As String, _
ByVal firstLienZipExtenstion As String, ByVal SecondLienStreet As String, ByVal SecondLienCity As String, _
ByVal mailingCity As String, ByVal mailingstate As String, ByVal SecondLienZipCode As String, _
ByVal SecondLienZipExtenstion As String, ByVal mailingStreet As String, ByVal mailingZipCode As String,
ByVal mailingZipExtenstion As String, ByVal firstLienDPBC As String, ByVal SecondLienDPBC As String,
ByVal mailingDPBC As String, ByVal secondLienSate As String, ByVal specialMailerCode As String,
ByVal salvageParts As String, ByVal mileageBrand As String, ByVal mobileDealerNumber As String, _
ByVal taxExemptIndicator As String) ( 76 parameters)
 
DeleteTitleRegistrationBE.New(
ByVal PlateType As Integer, ByVal IsTempPlate As Boolean, ByVal PlateBackground As Integer, ByVal PlateSubType As Integer, _
ByVal VehicleUsage As Integer, ByVal IsPersonalized As Boolean, ByVal IsHandicap As Boolean, ByVal IsVanity As Boolean, _
ByVal ResidentAddressStreet As String, ByVal ResidentAddressCity As String, ByVal ResidentAddressState As String, ByVal ResidentAddressZipCode As String, _
ByVal ResidentAddressZipCodeExt As String, ByVal CustomerNamePartOne As String, ByVal CustomerNamePartTwo As String, ByVal DriverLicensceNumber As String, _
ByVal PassportIndicator As String, ByVal PlateQuantity As String, ByVal DealerNumber As String, _
ByVal RegistrationFee As Decimal, ByVal TabNumber As String, ByVal PlateNumber As String, _
ByVal PreviousPlateNumber As String, ByVal RegistrationDuration As Short, ByVal RegistrationExpirationDate As String, _
ByVal ReplacementTabIndicator As String, ByVal ResidentAddressCountyCode As String, ByVal ModelYear As Short, _
ByVal Make As String, ByVal ManufacturerID As String, ByVal VehicleBodyStyleCode As String, ByVal Weight As Long, _
ByVal InstantTitleCode As String, _
ByVal ResidentAddressDPCCode As String, ByVal VehicleType As Integer, ByVal CustomerType As String, ByVal OwnershipType As Integer, ByVal RegistrationType As Integer) ( 38 parameters)
 
 
AuditReportBPC. GetAuditDataDetail(ByVal auditID As String, ByVal RequestorName As String, _
                          ByVal Branch As String, ByVal WorkStationName As String, _
                          ByVal ChannelType As Integer, ByVal RequestorType As Integer, _
                          ByVal formName As String, ByVal controlName As String, _
                          ByVal cartID As Integer, ByVal completedTXNID As Long, _
                          ByVal inProgressTXNID As Integer, ByVal customer_Number As String, _
                          ByVal vin As String, ByVal startDate As DateTime, ByVal endDate As DateTime, _
                          ByVal firstName As String, ByVal middleName As String, ByVal lastName As String, _
                          ByVal plateNo As String, ByVal dateOfBirth As DateTime, ByVal ssn As String, _
                          ByVal sortOrder As String, ByVal isAllBranchesSelected As Boolean, _
                          ByVal binarySum As Integer, ByVal IISSessionID As String, _
                          ByVal ReqIPAddress As String, ByVal ReqBrowser As String, _
                          ByVal ReqBrowserVersion As String, _
                          Optional ByVal XMLMethodName As String = "Audit") _
                          As DataSet Implements IAuditReportBPC.GetAuditDataDetail (29 parameters))
 
 
DuplicateTitleRegistrationController.BulkVehicleDetails
                                          (ByVal aNext As String, ByVal aPrevious As String, ByVal aCancel As String, ByVal aReset As String,
                                           ByVal aSearchVehicle As String, ByVal aSelectedVehicle As String, _
                                           ByVal aRemoveVehicle As String, ByVal aGridName As String, ByVal aAction As String, ByVal aPage As String, _
                                           ByVal VehiclesGrid As String, ByVal ORVVehiclesGrid As String, ByVal WaterCraftVehiclesGrid As String, _
                                           ByVal MobileHomesGrid As String, ByVal SearchedVehiclesGrid As String, _
                                           ByVal SelectAllVehiclesGrid As String, _
                                           ByVal RemoveAllVehiclesGrid As String, _
                                           ByVal SelectAllORVVehiclesGrid As String, _
                                           ByVal RemoveAllORVVehiclesGrid As String, _
                                           ByVal SelectAllWaterCraftVehiclesGrid As String, _
                                           ByVal RemoveAllWaterCraftVehiclesGrid As String, _
                                           ByVal SelectAllMobileHomesGrid As String, _
                                           ByVal RemoveAllMobileHomesGrid As String, _
                                           ByVal SelectAllSelectedVehiclesGrid As String, _
                                           ByVal RemoveAllSelectedVehiclesGrid As String, _
                                           ByVal SelectedVehiclesGrid As String, _
                                           ByVal aFormCollection As FormCollection) As ActionResult (27 parameters)
 
 
I have looked around on what is the right number of parameters for a good API design. Numbers like 3, 5 and 7 popped up, but is pretty much common consensus that >= 10 are considered as too many, whither it is in Java or .Net program…
 
 
This shows how far away we are from having a high quality code base… maybe we can spend some time on these long parameter list methods, at least we should stop making them longer than 10.
 
 
There are 2 common approaches in shortening the parameter list.
<![if !supportLists]>1.       <![endif]>Replace the parameter with a method call. What it says is that instead of letting the caller provide the value for the parameter, the member makes a call to get the value inside the implementation of the member.
<![if !supportLists]>2.       <![endif]>Intrude Parameter object. What it says is that instead of passing these values through big list of individual parameters, we pass these values through a parameter object; the parameter object is an instant of a class or a structure, which has many members in it.  However, when we blind parameters into a class or a structure, we need to make sure we only blind these related parameters together.  Take an example here:
 
GenerateAuditReport (parameter as AuditReportParameter)  where AuditReportParameter is a structure defined as following:
 
Public Structure AuditParameter
    Public AuditID As String
    Public RequestorName As String
    Public Branch As String
    Public WorkStationName As String
    Public ChannelType As Integer
    Public RequestorType As Integer
    Public FormName As String
    Public ControlName As String
    Public CartID As Integer
    Public CompletedTXNID As Long
    Public InProgressTXNID As Integer
    Public CustomerNumber As String
    Public VIN As String
    Public StartDate As DateTime
    Public EndDate As DateTime
    Public FirstName As String
    Public MiddleName As String
    Public LastName As String
    Public PlateNo As String
    Public DateOfBirth As DateTime
    Public SSN As String
    Public SortOrder As String
    Public IsAllBranchesSelected As Boolean
    Public BinarySum As Integer
    Public IIISSessionID As String
    Public ReqIPAddress As String
    Public ReqBrowser As String
    Public ReqBrowserVersion As String
    Public XmlMethodName As String
End Structure
 
<![if !supportLists]>3.       <![endif]>If you look at these long parameter list examples above, 2 of them are used as constructor. Starting from Visual Studio 2008, MS offered an easy way to instantiate object, it call object initializer. Take an example here. With this syntax, these long parameter list constructors are no longer needed. You might notice in the syntax bellow, you provide value for property by property by name, instead of depending on parameter order when you use constructor. When your parameter list is long, and you provide values for them based on order, you could easily make some mistakes that are hard to debug, especially they are all with the same type. So it is highly recommended to use this object initializer syntax.
 
Person = New Person() With {.FirstName = "John", .LastName= “Doe”, SSN = “123456789”}

No comments:

Post a Comment