Monday, June 23, 2014

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.
 

No comments:

Post a Comment