Monday, June 23, 2014

FW: Bad smell in code-- Temporary Fields

this is how the book says about temporary fields:

sometimes toy see an object in which an instance variable is set only in certain circumstances. such code is difficult to understand, because you expect an object to need all its variables. Trying to understand why a variable is there when it doesn't seem to be used can drive nuts.

From OOP point of view, object module (entity model) should reflect the business model it is representing. Throughout the life time of each object, all its properties should be set and valid. Setting value for one specific property in some specific circumstances only and use it in some other specific circumstances is bad practices in programming.

In our code base, let's focus on Business Entity projects where all classes with "BE" suffix is hosted. let's randomly pick up a class, CustomerBE or DrivinglicenseBE, ask the following questions with each read/write property of them:

1) is it independent of others?
2) is it valid from the time the object is instantiated till the object is out of scope?
3) when this object is persisted to the database, is this property used ?
4) when this object is retrieved from the database, is this property set? ( other than lazy loading for performance reason)
5) at any point of time when accessing this property will not result in "Object reference not set" exception?

if you get "yes" to all of the questions here, you are good, otherwise, it is possible this property could be considered as "temporary"...


take an example, in DrivingLicenseBE: take a close look at the following properties:

AAMVADenialMessage
IsSeasonalCDL
IsCDL
IsOutofStateCDL
BirthCertificateRequiredForDCHBirth
USPassportData
HasRuleMatched
PrivID
InvalidDLTxnMsgID
TxnCredentialID
CleanLearnerPermitRecord
LearnerPermitDuration
ImageHistory
KnowledgeExamDetails
TestingRequirementHistory
IsRoadSkillTestWaived
ExistingRenewDLPrivileges
PreviousPrivID
VisionScreeningResult
ActualExpireDate


This class is as large as 5700 LOC, I only scanned up first 1000 LOC. and I got a list of 20 peoperties... some of them may not been used at all, some of them may only referenced in UI or projects in other solutions. I am sure you will find some Tempporary Fields in this list, you might be able to find some unused properties here.... and I am also sure that you can find lot of them in other BE classes we have.

Those kind of temporary fields or unused properties make programming and troubleshooting very diffcult and unpleasant . "they drive us, as programmers nuts"


enough said about the temporary field and how many we have them. question to be asked: how to fix them?

1) if it is not used, take them out.
2) if it is only used one specific circumstances , try to replace it with parameter.

preventive measure:

remember to have a property you are committed to maintain its legitimacy throughout the lifetime of the object. one should be seriously consider before making that move. when you want to add a read/ write property, ask yourself: Am I ready to make this commitment ?

No comments:

Post a Comment