Extinguishing the null plague: a story of refactoring towards the Null Pattern

The system in our story is written in Java, and a little analysis revealed that 20% of the methods have something to do with null. one of its core components relies on retrieving various properties from the database. These properties can be of different types ranging from simple strings to composite properties.

The initial implementation modeled these values through a hierarchy of Property classes, as sketched below.

Property.png

The properties were used in many places throughout the system by means of an engine that hid the internal implementation. As it happened, when no value was defined for a property, the engine would work with raw null. This was enough for the runtime of the system.

After a while, a new use case required an editor for manipulating properties. This editor required access to the internals of the engine, and for each type of property it had to provide a dedicated input box. A straightforward application of the Visitor pattern (see below).

Visitor.png

Through the Visitor infrastructure, the editor could easily create a dedicated widget for each property type. It worked well, except for a little glitch: when the value was null, there was no object, and thus, the visitor could not work (and the fact that null in Java is not an object did not help either). The solution for making things smooth was the introduction of a Null Pattern.

As a consequence, we proceeded to introduce the UndefinedProperty. To make use of it, we needed to see where the properties are initialized with null and replace that with new UndefinedProperty(). We found one place easily just by inspecting the code, but we still need to make sure there are no other places that initialize properties variables with null. We could have just searched for null everywhere in the system, but that would have not been useful given that we found several thousand null usages throughout the system. We needed a more accurate query, and we used Moose:

| superClass variablesOfPropertyType |
superClass :=  self allModelTypes entityNamed: #'com::example::Property'.
variablesOfPropertyType := superClass withSubclassHierarchy flatCollectAsSet: #structuresWithDeclaredType.
variablesOfPropertyType select: [ :each |
     (('*', each name , ' = null') match: each belongsTo sourceText) ].

The query above identifies all variables that are of our Property type, and checks whether there is an initialization with null somewhere in the surrounding method. There were no other places.

Undefined.png

The tests were green. All looked fine. Almost.

The only remaining problem was that we needed to ensure identify the places that relied on an unknown property being null. In other words, we had to look for code like property ==null or property != null. Moose made it easy again.

| superClass variablesOfPropertyType |
superClass :=  self allModelTypes entityNamed: #'com::example::Property'.
variablesOfPropertyType := superClass withSubclassHierarchy flatCollectAsSet: #structuresWithDeclaredType.
variablesOfPropertyType select: [ :each |
     (('*', each name , ' != null*') match: each belongsTo sourceText) or: [
      ('*', each name , ' == null*') match: each belongsTo sourceText ] ].

We found several places that relied on such checks and we simply had to replace them with isUndefined(). The reason why the tests were green was that the fixtures simply did not include cases in which properties were not defined. Of course, before replacing the code, we first wrote a couple of red tests to ensure that we capture those cases for the future. Once we modified the code, the tests turned green again.

Now we were confident that the system will not crash, but we were still not ready just yet: we still need to update the comments to reflect the new contract. Thus, we had to search every mentioning of null in the comments of the methods returning Property values:

| superClass methodsReturningPropertyType |
superClass :=  self allModelTypes entityNamed: #'com::example::Property'.
methodsReturningPropertyType := superClass withSubclassHierarchy flatCollectAsSet: #behavioursWithDeclaredType.
methodsReturningPropertyType select: [:each |
     '*null*' match: (each comments anyOne sourceText ]

And we were done.

The whole exercise took a couple of hours. At the beginning, the task seemed both expensive and risky. However, when approached systematically, it turned out both to be cheap and to pose no real problem after all. All of this happened because we used the assessment tools and techniques integrated deep in the development process.

Posted by Tudor Girba at 27 July 2013, 1:42 pm with tags story, spike, moose link
|