Bad Smell in code - lack of constructors (Object reference not set)

For some people who are not too much into programming, you may not know much about constructor, but I am sure if you are in this project for a while, you surely know something about errors with the message of "Object reference not set". Most of "object reference not set" are caused by lack of constructor, the rest of it is caused by "Nothing", in my article titled "Nothing, a necessary evil" I addressed of usage of "Nothing" ( null in c#) issue, in this article I am going to talk about constructor.

Before proceed into the detail of constructors, let me show you some statistics on "Object reference not set" exception:

In production system, from July-27-2011 to September 10th 2013, Just in the Web presentation layer alone, we recorded 58476 exceptions. Of which 14458 is "Object reference not set". Excluding 4080 repeated ones, "Object reference not set" error contributes 26% of all exceptions logged.

now, let's look at the codebase we have,

Just take DataEntity as an example, there are 1137 classes in the project, among which 890 classes do not have any constructor. which means 78% of our entity classes do not have even one constructor. within 22% of the classes with constructor, some of them are empty ones. like the one in AddressBE, AuditEventBE, AuditTransactionBE, BatchCPCOutOfStateLetterBE, BatchErrorRecordsBE, PINRenewalUpdateRequestBE, MetaDataBE, BusinessTableBE... the constrcutor is something like

Public Sub New()
End Sub

I am sure you can find lots more than I did here.


so the real % of classes lack of constructor could be more than 80%.

Now let's go back to our code to see how we cope with this "lack of constructor" smell. you guessed it right! we check if it is Nothing before access it"

If batchRequestErrors IsNot Nothing Then
If batchSalvageAgentExist Is Nothing Then

Just in BO project alone, we have 856 "IsNot Nothing" checks and 429 "Is Nothing" checks.

There are total 40291 line of code in BO project. Based on the figure here, we could calculate that there are 3 "Nothing" checks in every 100 line of code. When there is an "Object reference not set" defect filed, we find the spot and add a Nothing check around it... This is not proper way of fixing "Object reference not set" defects. Instead we should find out the source of "Nothing" value, and find out if it a valid Nothing or invalid Nothing. Does the class have a constructor ? Is the constructor failed to instantiate the instance?

With the way we do, we end up with 2 bad smells in our code and one bad taste in user experience. In the code, we have "lack of constructor" and "excessive of nothing check". in user experience , we have many "Object reference not set" STEs in the log. I am sure you want to do better than that!

Fortunately, the fix to this bad smell is magically simple: Add a default constructor to all classes, and in the constructor, call its base class constructor and insatiate all its members. The following is an example:

Public Sub New()
Mybase.New()
Me.mID = 0
Me.mVehicleID = 0
Me.mDescription = String.Empty
Me.mVehiclePurchaseDate = Date.MinValue
Me.mRTSSurvivalshipCode = String.Empty
Me.mRTSSurvivalshipDesc = String.Empty
Me.mIsUnderCover = False
Me.mIsOrConjunction = False
Me.mTitle = New TitleBE()
Me.mRegistration = New RegistrationBE()
Me.mVehicleCost = New VehicleCostBE()
Me.mOwnerships = New Collection(Of OwnershipBE)
End Sub


2 years ago, I identified this bad smell and I published a guideline on usage of constructor... along the way of my code review, I personally added lots of constructors to the classes. I thought to put together this article to refresh the call of using constructor.