Monday, June 23, 2014

FW: Compact DB, Smart Client and Application Caching

Around June 2004, Microsoft released Smart Client Architecture and Design Guide, in the article, Microsoft listed 3 important characterizes of Smart client application:
 
  1. It offers rich user experience like normal window application, unlike normal web application
  2. It works in disconnected mode. ( MS call it Occasionally Connected)
  3. It can be deployed by “no-touch deployment” technology and capable of self-update
 
An example would be like a salesman carries his laptop walking into his customer office, he should be able to show the product catalog, take orders  when talking to his customer  when the laptop is disconnected.  However the orders will be uploaded to the ERP or MRP system, and you should be able to synchronize the product catalog  when he get back to his office and connect the laptop to the company network.
 
To make it possible for the application to function in disconnect mode, the application needs to stored it transactional data and reference data…  To facilitate this kind of functionality, Microsoft introduced concept of Compact DB and database synchronization framework.
 
On the other end, around April 2003, Microsoft release Caching  Application Block as a member of Enterprise Library ( in fact, Avanade is the company behind all enterprise Library development, and I happen worked for Avanade for 3 years from 2004 to 2007). http://msdn.microsoft.com/en-us/library/ee957904.aspx
 
This is what it says about caching, “Caching techniques are commonly used to improve application performance by storing relevant data as close as possible to the data consumer, thus avoiding repetitive data creation, processing, and transportation.”
 
Okay, enough stuff about Microsoft technologies. Let’s move on to our life, BAM. First of all, I have reviewed all non-functional requirements of MI VBAM, it was not our goal to design and develop a smart client. It was not our goal to make it still functional in disconnected mode.    But we are using Compact DB and database synchronization framework.
 
If you look at the data that is been synchronized, you will find more than 70% of data is not supposed to be changed, the other 30% is just some reference data, there is no 2 way synchronization needed. no transactional data is involved in the synchronization process. Basically, we use it as caching solution.
 
To give you some feel about what kind of data we are loading to the compact DB and involved in the synchronization process, I list some tables ( not complete list, in total there 333 tables ) as following:
 
  1. REF_CUSTOMER_TYPE
  2. REF_ID_TYPE
  3. REF_CONTACTS
  4. REF_ADDRESS_TYPE
  5. REF_STATE
  6. REF_CITY
  7. REF_SUFFIX_CODES
  8. REF_COUNTRY_CODE
  9. REF_INTL_MILATARY_STATE
  10. REF_INTL_MILITARY_CITY
  11. REF_CONTACT_TYPE
  12. REF_MODE_TYPE
  13. REF_INDIVIDUAL_CHARACTERISTICS
  14. REF_BUSINESS_ROLES
  15. REF_CITY_ZIPS
  16. REF_COST_EXEMPT
  17. REF_DAMAGE_REASON_CODES
  18. REF_FUEL_TYPE
  19. REF_GROUP_TYPE
  20. REF_IMAGE_TYPE
  21. REF_NAME_TYPE
  22. REF_PREFIX_CODE
  23. REF_SALE_TAX_OUT_OF_STATE
  24. REF_SALE_TYPE_CODE
  25. REF_STOP_ACTION
  26. REF_TAX_EXEMPT
  27. REF_VEHICLE_BODY
With that, I am propose the following suggestions
 
  1. Review this 333 table to remove these tables that data is not manageable by user. A clear cut would be if we defined a enum in our code, for the same data, we do not even need the data in the server database… please refer to the link https://external1.collaboration.hp.com/external/MIBAM-R1/MI%20BAM%20Cookbook/Enum%20types%20and%20Ref%20tables.aspx for some detail.
  2. do away with Compact DB and synchronization framework.
  3. Employ Caching  Application Block to cache the data need to be cached in both Client (Process Controller) and server side (BO layer)
 
 
Just so you know,  Caching  Application Block is the application block that is most easy to be employed… 10 minutes would be enough for you to know how to use it… by doing this, the problem we are facing in Compact DB and database synchronization framework will no longer exist…
 

Duplicated Code and how to eliminate them

Number one in the stink program is duplicated code. If you see the same code block in more than one place, you can be sure that your program will be better if you find a way to unify them.
 
. When you see the same block of code in two methods of the same class, extract the code block into a private method and invoke the method from both places.
 
.when you see the same block of the code in 2 sibling sub-classes, extract the code block into a protected method in the base class and invoke the method in both places.
 
.When you see the code block are doing the same thing but in different way, extract the better code block into a private method and invoke the method in both places.
 
.When you see the duplicated code blocks are doing the same but on different target, extract the code block into a private method with some parameters, and then invoke the method in both places with different parameter values.
 
.When you see a duplicated code block in 2 unrelated classes, you have a few options. You need to decide which one make more sense from business point of view.
a.       Abstract the duplicated code blocks into an abstract base class and let the 2 classes inherit from it. (Be its sub-class). In another word, make them related by be the sub classes of the same base class.
b.      Encapsulate the code block in third class as internal/public read only property or method and invoke it in both place. This will also reduce class coupling.
c.       Abstract the code block into a public method in one class and let other class invoke the method in the hosting class. 
 

RE: bad smell code # 2 Long Method

I took a look at a codebase reviewed before and I found that the top 3 longest methods are as following:
Interface. InsertNADAData(String) As Boolean ( 774 LOC)
 
RegistrationNewUserControl. ValidateControls(Integer) As Boolean ( LOC 629)
 
RenewalRegistrationController. VehicleDetails(String, String, String, String, FormCollection, String, String, String, String, String, String, String, String, String, String, String, String, String, String) As ActionResult ( LOC 617)
 
If you happen to step into any of these members, you would agree with me when I say these code blocks are smell bad. They are very difficult to understand let alone maintaining them or extend them….
 
Let’s say how the experts say about long methods:
 
The object program that lives best and longest is these with short methods. Programmers new to objects often feel that no computation ever takes places that object programs are endless sequence of delegations. When you have lived with such a program for a few years, however, you learn just how valuable are those little methods are.  All of these payoffs of indirection – explanation, sharing and choosing. All supported by little methods. …. Since the early days of programming, people have realized that the longer the procedure, the more difficult to understand.
 
Okay, we accepted the call that “short method is good, long method is bad” but given the code base we have, how we gradually shorten the methods of thousands class in our code base?
 
.99% of the time, all you have to do to shorten a method is Extract. Find a part of the method that seems to go nicely together and make it a new method.
 
.If you find that you have a method with lots of parameters, you can introduce parameter objects to group these parameters into groups. (I am sure you would like to try this method on the third example above)
 
.If you find that you have a method with lots of temporary variables, you can move the related code block from your method to the class that is close to it as a read only property or method. (I am sure you have seen many of such examples from my code review comments)
 
After you have done all above, if you still end up with a long method with lots of parameters and local variables, it is time to turn the method into a class by itself. After you do that, all you original local variable become fields of the new class, then you can decompose the method into other small methods on the same object.
 
How do you identify the clump of code to extract? A good technic is to look for comments. They often signal this kind of semantic distance. A block of code with a comment that tells you what it is doing can be replaced by a method whose name is based on the comments. Even a single line is worth extracting if it needs explanation.
 
Conditionals and loops also give signs for extractions. If you have a complicated conditional (if-then-else) statement, extract methods from the <condition part>, <then part> <else part>. With loops, extract the loop and the code within the loop in its own method.

FW: bad smell code # 5 Divergent Changes

We structure our software to make changes easier; after all software is meant to be soft. When we make a change we want be able to jump to a single clear point in the system and make the change. When you cannot do this you smell one of 2 closely related pungencies, Divergent Changes and short gun Surgery
 
Divergent changes occurs when one class is commonly changed in different ways for different reasons. If you look at the class and say “well, I will have to change these 3 methods every time I get a new database, I have to change these 4 methods when month end process changes” you likely have a situation in which 2 classes is better than one. That way each class is changed only as a result of one kind of changes.
 
Now, go back to our code base, imagining that state of Michigan changed some policy related to driving license, the minimum age to be able to apply for driving license is not 16 and half years old, instead, it is 16.  How many places in our code we need to change?  ( short gun Surgery)
 
Take a look at TransactionBO class (it is 9320 LOCs in size), ask ourselves are the following statements true: this class contains business logic all DriverLicense, Vehicle, Placard, title and so on…   We have about 15 different TransactionBEs (TitleRegBE, DrivingLicenseBE , DocumentAuthenticationBE) and over 100 different transaction types. TransactionBO class has related logic on all of them. You will probably a group of methods for each transaction BE in this class. Sometime, you will find within a method, there is some code like “Select Case transaction.TypeCode”  or If TypeOf Transaction Is TitleRegBE”
 
When you look at TransactionBO, would you say something like “I need to change these methods if the process for renew Driver License changes, I need to change those methods if the process for register a newly purchased vehicle changed , the same goes for title, placard….”
 
Ok, we know there are this type smell in our code base, how to get rid of it. There are 2 different ways. One is consider surgical way one is consider non-surgical way.   Let me describe them in detail here:
 
Surgical way:
You scan through the class, and identify the members that is related to some specific type of operations (e.g. DrivingLicense operations), extract them in to a new class and change the rest of the codebase to use the new class instead of original class.  In this way the effect is immediate, the impact is large. So the effort and risk associated to it is large.
 
No Surgical way:
You add a new class (DrivingLicenseBO) for a specific type of the operation (e.g. DrivingLicense operations), and you let the original class (TransactionBO) be its base class. In the beginning DrivingLicenseBO is empty.  Then you make an announcement say “from now on, when you are doing DrivingLicense related operation you use DrivingLicenseBO instead of TransactionBO” as it is now, the effect is the same, as DrivingLicenseBO is a empty sub class of TransactionBO.  So other programmer including yourself, start to change their code to use DrivingLicenseBO instead of TransactionBO for DrivingLicense related operations. When this process is (almost) completed, you then can start to move methods specific DrivingLicense related operations to from TransactionBO to DrivingLicenseBO. In this way, the initial effort and risk associated to the change is much smaller, but it will take long time to complete the migration process.  
 
The end result of these 2 approaches is the same. The only difference is the process. Projects choose different approach based on their situations. If it is affordable for project to endure this kind of surgery operation, and you are confident with what you are doing, you should take up the opportunity to conduct the surgical operation. If not. You can at least start the process using the non- surgical process.
 
One important point to note that, when a class has “Divergent Changes” smell, it most likely will also has “large class”… when we eliminate “Divergent Changes” smell; we also work towards eliminating   “large class” smell.

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”}

bad smell code # 3 Large class

I took a look at our current code base; I found that the top 3 largest classes are as following:
CustomerDetailsForm1 (12261 LOC)
 
TransactionBO (LOC 9066)
 
DLPreRequisiteUserControl ( LOC 8124)
 
I have looked around on what is the right size of a class in term of LOC, there are many numbers popped out, but it is pretty much common understanding that one thousand LOC can be considered as too large, whither it is in Java or .Net program…
 
One of the articles I found stated the following:
<300 lines: fine
300-500 lines: reasonable
500-1000 lines: maybe ok, but plan on refactoring
>1000 lines: definitely refactor
With that I took a second look at our code base. Out of about 6000 classes,  
 
> 5000: 11
Between 5000 and 2000: 58
Between 2000 and 1000: 101
 
This shows how far away we are from having a high quality code base… maybe we can set a short term goal as make them all under 5K. Now; let’s look at how to break big classes into smaller ones.
 
When a class is trying to do too much, it is often shows up as too many members (methods) and/or too many instance variables, when a class has too many members or instance variables, duplicated code cannot be far behind.
 
There are 3 common approaches in making a class smaller.
<![if !supportLists]>1.       <![endif]>Make them smaller by eliminating duplicated code. Please refer to the first 2 articles addressing duplicated code and long methods.
<![if !supportLists]>2.       <![endif]>Push some logic that does not belong to it. If we let a class to do something that is not part of its responsibility, we could quickly put on weight on the class. The goal of this process is to push the logic to where it belongs
<![if !supportLists]>a.       <![endif]>For UI class, ask yourself should this logic to be implemented in UI, is Business layer or entity layer a better place for it?
<![if !supportLists]>b.      <![endif]>For Data Access class, ask yourself does it related to data access or is it some kind of business logic should be in Business layer or entity layer?
 
<![if !supportLists]>3.       <![endif]>Break it up into smaller class. If we try to let a class to do too many things, the class could be very big.  One of the OOP principles is SoC (Separation of Concerns). Following that principle, we should clearly define the boundary of each class. Take an example. Let’s say we have a business class call Customer. It is reasonable to assume this calls its respective Data Access class to access database when it need to retrieve or persist data, and it would not provide functionality which is not directly related to customer domain. When a class has grown too big, we can redefine the boundary of the class so that the large class can be broken into smaller ones by grouping its functionality into a few groups. Take an example, if we break our TransactionBO into multiple smaller classes, one for Title and Registration transactions, one for Driving License transaction, one for Plate Card…. etc.. Of course, we can put some common functionality into a base class so that they are available in all its subclasses. The LOC of these classes will be much smaller than what it is now in TransactionBO.
 
If your entity class is too big, the most common way is to extract a port of the entity into another entity class reference to it.
 
If your GUI class (form or user control) is too big, first thing you want to do is to investigate whether there are business logic implemented in UI layer which is better in business layer or entity layer. If that is not enough, you would want to investigate opportunities to break some potation of this UI into a user control by itself.
 
Another observation I have is during the development phase (only in development phase, NOT in stabilization phase), if a class is too hot, many developers are fighting to check it out for their work, or there are many change sets checked in on the same class, it is an indication that it might do ourselves good to break the class into small ones.

FW: Bad smell in Code #6 Shotgun Surgery

Shotgun surgery is similar to divergent change but it is the opposite. You whiff this smell when every time you a kind of change, you have to make a lot of little changes to a lot of different classes. When the changes are all over the place, they are hard to fine and it is easy to miss some, which leads to defects.
 
Now, go back to our code base, imagining that state of Michigan changed some policy related to driving license, the minimum age to be able to apply for driving license is not 16 and half years old, instead, it is 16.  How many places in our code we need to change?
 
Assuming the state changes the driving license policy in view immigration law reform. The new rule says, if you are “undocumented immigrant, and you are able to prove that you entered the country before or on Jan 1st 2000, you are allowed to apply for state driving license” how many classes we need to change to make this happen in our system?
 
Assuming the state changes its policy on eCheck. Based on the new policy, only vehicles of 5 years or older need to submit eCheck report instead of 4 years for renewal of registration. How many places we need to change to make this happen in our system?
 
I won’t be surprise, if you tell me there are a few forms, asp pages, a few user controls and handful of classes at server side maybe data in some database tables need to be touched to do implement these business changes. If we design it and program it right, there should be one place to change in code and/or one place to change in database. Anything more than two will be considered as too many.
 
These are evidences of existence of “Shotgun Surgery” smell in our code…
 
Okay, we have “shotgun surgery” smell in our code, how to get rid of it?
 
First of all we need to establish the consensus that all related changes should be in one class.
Then identify an existing class or create a new class for that purpose.
After that, slowly move all related business logic (public members, private members) to this class.
When a method is moved to this class from various classes, either turn the old methods into simple delegations (for migration purpose) or remove them altogether.  
When all a class is doing is delegating the calls to someone else, the class can be eliminated.

FW: bad smell code # 5 Divergent Changes

We structure our software to make changes easier; after all software is meant to be soft. When we make a change we want be able to jump to a single clear point in the system and make the change. When you cannot do this you smell one of 2 closely related pungencies, Divergent Changes and short gun Surgery
 
Divergent changes occurs when one class is commonly changed in different ways for different reasons. If you look at the class and say “well, I will have to change these 3 methods every time I get a new database, I have to change these 4 methods when month end process changes” you likely have a situation in which 2 classes is better than one. That way each class is changed only as a result of one kind of changes.
 
Now, go back to our code base, imagining that state of Michigan changed some policy related to driving license, the minimum age to be able to apply for driving license is not 16 and half years old, instead, it is 16.  How many places in our code we need to change?  ( short gun Surgery)
 
Take a look at TransactionBO class (it is 9320 LOCs in size), ask ourselves are the following statements true: this class contains business logic all DriverLicense, Vehicle, Placard, title and so on…   We have about 15 different TransactionBEs (TitleRegBE, DrivingLicenseBE , DocumentAuthenticationBE) and over 100 different transaction types. TransactionBO class has related logic on all of them. You will probably a group of methods for each transaction BE in this class. Sometime, you will find within a method, there is some code like “Select Case transaction.TypeCode”  or If TypeOf Transaction Is TitleRegBE”
 
When you look at TransactionBO, would you say something like “I need to change these methods if the process for renew Driver License changes, I need to change those methods if the process for register a newly purchased vehicle changed , the same goes for title, placard….”
 
Ok, we know there are this type smell in our code base, how to get rid of it. There are 2 different ways. One is consider surgical way one is consider non-surgical way.   Let me describe them in detail here:
 
Surgical way:
You scan through the class, and identify the members that is related to some specific type of operations (e.g. DrivingLicense operations), extract them in to a new class and change the rest of the codebase to use the new class instead of original class.  In this way the effect is immediate, the impact is large. So the effort and risk associated to it is large.
 
No Surgical way:
You add a new class (DrivingLicenseBO) for a specific type of the operation (e.g. DrivingLicense operations), and you let the original class (TransactionBO) be its base class. In the beginning DrivingLicenseBO is empty.  Then you make an announcement say “from now on, when you are doing DrivingLicense related operation you use DrivingLicenseBO instead of TransactionBO” as it is now, the effect is the same, as DrivingLicenseBO is a empty sub class of TransactionBO.  So other programmer including yourself, start to change their code to use DrivingLicenseBO instead of TransactionBO for DrivingLicense related operations. When this process is (almost) completed, you then can start to move methods specific DrivingLicense related operations to from TransactionBO to DrivingLicenseBO. In this way, the initial effort and risk associated to the change is much smaller, but it will take long time to complete the migration process.  
 
The end result of these 2 approaches is the same. The only difference is the process. Projects choose different approach based on their situations. If it is affordable for project to endure this kind of surgery operation, and you are confident with what you are doing, you should take up the opportunity to conduct the surgical operation. If not. You can at least start the process using the non- surgical process.
 
One important point to note that, when a class has “Divergent Changes” smell, it most likely will also has “large class”… when we eliminate “Divergent Changes” smell; we also work towards eliminating   “large class” smell.
 

A technic to eliminate select-case structure

Supposedly I have a base class call Vehicle ( in our case, it would be TransactionBE) and I have a sub class of it call Car and another call Van. you may add another Bus, Truck to the list...

Let's say I have a class call Driver (in our case it would be TransactionBO), inside this class I have an overloaded method call Start, inside the start methods, it does different thing for different type of vehicle.

I have a method call Drive which take Vehicle as parameter.as the name suggests, it drive the vehicle. inside the method, the first step is to get the vehicle started. because the Start method is an overloaded one, what we need to do is to check the type of vehicle and then cast it to respective type and call the start method.



Public Class Vehicle

End Class

Public Class Car
Inherits Vehicle
End Class

Public Class Van
Inherits Vehicle
End Class


Public Function Start(ByVal vehicle As Van)
Return "Van gets started"
End Function

Public Function Start(ByVal vehicle As Car)
Return "Car gets started"
End Function

Public Function Start(ByVal vehicle As Vehicle)
Return "the vehicle gets started"
End Function

Public Function DriveSelectCase(ByVal vehicle As Vehicle)
Select Case True
Case TypeOf vehicle Is Van
Return Start(DirectCast(vehicle, Van))
Case TypeOf vehicle Is Car
Return Start(DirectCast(vehicle, Car))
Case Else
Return Start(vehicle)
End Select
End Function

in this case, I have 3 different classes, in our case, we have about over 10 different sub class of TransactionBE, so our case statement would be longer than what I have here. but you get the point, right?

well, there is a way to replace the select-case structure with one line. this is how you do it:


Public Function Drive(ByVal vehicle As Vehicle)
Return Start(Convert.ChangeType(vehicle, vehicle.GetType()))
End Function


the benefits are as following;

1) write less code, less in most of time means better.
2) it is faster, the code does not need to go through the select case logic
3) as we eliminated the select-case structure, the cyclomatic complexity of the method is reduced. in this case, the cyclomatic complexity of function without Select-case structure is 1 and that of the function with select-case structure is based on how big is the structure , in my example, it is 3.
4) less bug, suppose in DriveSelectCase method, we mismatch the Type name in the code like the following, the compiler does not know, you guessed right, you would get a run time error. while in the new method, this problem is eliminated because you do not need to type the type name twice.

Select Case True
Case TypeOf vehicle Is Van
Return Start(DirectCast(vehicle, Car))
Case TypeOf vehicle Is Car
Return Start(DirectCast(vehicle, Van))
Case Else
Return Start(vehicle)
End Select