Testing for the presence of a value

Back to some geek content... How often have you seen code like the following?

if ( foo.getBar() != null ) { /* use the value */ }
The code is testing if the foo object has a bar value. This usage seems innocuous, but it is not. It both hinders readability and refactoring. Let's address readability first.

The test is asking that the user of the class infer the presence of a value. However, it does not test for a value, but, instead, for the absence of a value, the none value. For readability, don't make the user guess what is happening but, instead, make what is happening explicit. Now, on to refactoring.

The none value test actually exposes an internal implementation of the class to the user of the class. What if a refactoring requires that the internal implementation of the none value change? A common change, for example, is for a null to be replaced with a sentinel value [1]. Should this happen then all uses of the class will have to be changed because the none value test is ineffective. Moreover, the ineffective test will not break the running code, but instead send execution down the then conditional path. Let's hope that no life or treasure is affected.

When you have all the use code under your control then the change is manageable. You will end up with an excessively sized patch that will dwarf the purposeful change. When the use code is not under your control then a ramification is that the simple, internal change is now a global external change. So what is to be done?

To improved readability and facilitate refactoring simply include an explicit test for bar.

if ( foo.hasBar() ) { /* use the value * }
The purpose of the test is clear and use does not expose an internal value.

[1] http://en.wikipedia.org/wiki/Sentinel_value

No comments: