Monday, June 23, 2014

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.

No comments:

Post a Comment