Thursday, December 19, 2013

Bad smell in Code Inappropriate Intimacy

this is what the book said about Inappropriate Intimacy:

sometimes classes become far too intimate and spend too much time delving in each other's private parts. we may not be prudes when it comes to people, but we think our classes should follow strict, puritan rules. over intimate classes should be broken up as lovers were in ancient days.... Inheritance often can lead to over intimacy. sub classes are always going to know more about the their parents ( base class) then their parents would like them to know.

There is one thing for sure, over intimacy is bad, which is the direct anti-pattern of Encapsulation. when you have this smell in the code, you will find it is difficult to change code, change in one class may lead to changes in many other classes. change in one user control may lead to change in many hosts( form or user controls) and hosts of the hosts and even more,,,, if you run code metrics, you will see high in class coupling, this is another direct anti-pattern of "high cohesion and low coupling".
let's take a look at our codebase to see where we are in this aspect:
the top 100 methods of class coupling are in the arrange of (38 to 94), more than 50% of them are in UI
the top 100 classes of class coupling are in the arrange of ( 119 to 378), more than 75% of them are in UI
CA1506: Avoid excessive class coupling is one of the warning message generated by Code analysis. MS message says "A class coupling above 40 indicates poor maintainability, a class coupling between 40 and 30 indicates moderate maintainability, and a class coupling below 30 indicates good maintainability." over 6410 classes in our code base, we have 586 classes with class coupling over 40. and 736 classes with class coupling over 30.


In UI project I know of the total number of classes is 831, while number of classes with coupling over 40 is 374.
it looks like the UI project is the main source of this bad smell. let's take a look at some examples code

Me.Address.DomesticAddUserControl.County.HideToCustomization()



This piece of code block is in a form. Address is an instance of user control AddressUserControl
inside AddressUserControl control there is an instance of another user control , USDomesticAddressUserControl

inside USDomesticAddressUserControl control, there is an instance of another control (part of DevExpress) , (DevExpress.XtraLayout.LayoutControlItem)
inside LayoutControlItem control , there is a method call HideToCustomization ()
Take form as level zero, you can tell the intimacy level is 3, anything happen to the API of all these user controls involved will affect the code in the form. this is a typical case of Inappropriate Intimacy.
take a look at this line of code

Me.Address.addressGroupControl.Text = "Address"

this code is in a form, in the form there is a user control AddressUserControl
inside AddressUserControl user control there is an instance of another user control, GroupControl
inside GroupControl there is a property named Text property.
in the form, the code reach out to AddressUserControl and go inside to GroupControl to set the Text property of the control. this is another example of Inappropriate Intimacy.
let's have another example:

Enough said about Inappropriate Intimacy, let's see how to deodorant this bad smell. specifically how to address it in UI project.
1) in a form/user control, all user controls it uses to be made as private, not accessible outside of it
2) the hosting form / user control access it through its public interface ( property, method and events)

in other classes, try to keep the public facing members to minimum. as our rule called "Secure by default". With those guidelines, the Inappropriate Intimacy smell in our code will be reduced.
if after doing all these , we still felt the intimacy is still high, there are other more advanced way to reduce the intimacy. the following are some technics:
1) move methods and fields so that they stay together ( in the same class)
2) change bidirectional association to unidirectional association between classes
3) if there are 2 classes sharing common interest, extract the commonality into a separate class
4) replace inheritance with Delegation
5) Hide Delegate to let another class act as go-between.

No comments:

Post a Comment