Monday, June 23, 2014

FW: Bad smell in code - Speculative Generality

"Oh, yeah, we will need it someday , we better build it now"

Is this familiar to you as a software engineer or software architect? this statement could be made when making a decisions as big as "The web front should be decoupled from other part of the system. the web front should be a plug and play for any similar system" . or as small as "I will need this parameter for this method even it always passing as constant, or "we need this class as a sub class even it does not do anything more than its base class. we will add more in the future"

Speculative Generality normally comes from "Speculative Requirement". based on Agile methodology or any feature based methodology as comparing to component based methodology) , no coding to be put in codebase for any requirement that is not confirmed yet. The motto is "we will build it when it is needed"

you smell it when you see some un-used parameters in class members; you smell it when you see a method was referenced with some constants value passing as parameter; you smell it when you see an base class or a nearly empty sub-class.
you smell it when you see an interface is only implemented by only one class. you smell it when you see nearly empty interface; you smell it when you see a few different interfaces with the identical API definitions.

if you are familiar with our code base, you would say "I have seen them all in it".

some of symptoms described in "Lazy classes" is coming from Speculative Generality. other possible source of "Lazy classes" could be 1) we may needed it some time ago, but we do not need it now, but we did not clean them up. 2) when more and more features added in, we did not refine our design, instead, we just copy and change... 3) unexplainable, did not think about it, they are just there.

Take an example, as of last code quality report, within 11 major Visual Studio projects in our system, there are
580 unused parameters in methods ( 146 in BO, 145 in UI). 758 unused local fields ( 214 in BO, 194 in CometWeb), 194 unused private fields, and 145 unused private methods. (36 in POSDataAccess, 35 in UI) and 25 in BPC) ,there are also many unused public methods.


the deodorant to this smell are very simple:

1. when you found an unused local fields, delete it
2. when you found an used private field or private method, delete it
3. when you found an empty class or interface, collapse it and then delete it
4. when you found unused parameter in a method call, overload the method without the parameter and retire the original one
5. when you found all reference to the method with the same constant value for the parameter. overload the method without the parameter and retire the original one
6. when you found a method check some values of a parameter , but none of its callers passing that value., remove the logic in the implementation.
7. when you found an empty project, remove it. when you found a nearly empty project merger it into other project with the namespace intact.
8. when you found an almost empty interface, collapse it to other existing interface
9. when you found a set of identical interface, unify them into one and delete the rest.

in summary,. do not build feature that is not needed.


the preventive prescription are:

1) we will build it when it is needed
2) when we build it, build it well. Dare to refectory the existing design to make it seamless. no cookie cutting.

No comments:

Post a Comment