By Kore Nordmann, first published at Tue, 03 May 2016 10:00:00 +0200
Download our free e-book "Crafting Quality Software" with a selection of the finest blog posts as PDF or EPub.
You can also buy a printed version of the book on Amazon or on epubli.
nullWhen doing code reviews together with our customers we see a pattern regularly which I consider problematic in multiple regards – the usage of null as a valid property or return value. We can do better than this.
Let's go into common use cases first and then discuss how we can improve the code to make it more resilient against errors and make it simpler to use. Most issues highlighted are especially problematic when others are using your source code. As long as you are the only user (which hopefully is not the case) those patterns might be fine.
One common use case is setter injection for optional dependencies, like:
class SomeLoggingService {
private $logger = null;
public function setLogger(Logger $logger) {
$this->logger = $logger;
}
}The logger will be set in most cases but somebody will forget this when using your service. Now a second developer enters the scene and writes a new method in this class using the property $logger. The property is always set in the use cases they test during development, thus they forget to check for null – obviously this will be problematic in other circumstances. You rely on methods called in a certain order which is really hard to document. An internal getLogger() method constructing a default null logger might solve this problem, but it still might not be used because the second developer wasn't aware of this method and just used the property.
In PHP versions < 7 a call to $this->logger->notice(…) will result in a Fatal Error which is particularly bad since the application can't handle this kind of errors in a sane way. In PHP 7 those errors are finally catchable but still nothing you'd expect in this situation.
What is even worse is debugging this kind of initialization. This is often even used together with aggregated objects which are required by the aggregating class. (You should not use setter injection for mandatory aggregates, but it is still used this way.) Let's consider the following code now:
class SomeService {
public function someMethod() {
$this->mandatoryAggregate->someOtherMethod(/* … */);
}
}When calling someMethod() and the property $mandatoryAggregate is not initialized we get a fatal error, as mentioned. Even if we get a backtrace through XDebug or change the code to throw an exception and get a backtrace it is still really hard to understand why this property is not initialized since the construction of SomeService usually happens outside of the current callstack but inside the Dependency Injection Container or during application initialization.
The debugging developer is now left with finding all occurrences where SomeService is constructed, check if the $mandatoryAggregate is properly initialized and fix it, if not.
All mandatory aggregates must always be initialized during construction. If you want a slim constructor consider a pattern like the following:
class SomeService {
public function __construct(Aggregate $aggregate, Logger $logger = null) {
$this->aggregate = $aggregate;
$this->logger = $logger ?: new Logger\NullLogger();
}
}The parameter $aggregate now is really mandatory, while the logger is optional – but it will still always be initialized. The Logger\NullLogger now can be logger which just throws all log messages away. This way there is no need to care about checking the logger every time you want to use it.
Use a so called null object if you need a default instance which does nothing. Other examples for this could be a null-mailer (not sending mails) or a null-cache (does not cache). Those null objects are usually really trivial to implement. Even it costs time to implement those you'll safe a lot time in the long run because you will not run in Fatal Errors and have to debug them.
null as Return ValueA similar situation is the usage of null as a return value for methods which are documented to return something else. It is still commonly used in error conditions instead of throwing an exception.
It is, again, a lot harder to debug if this occurs in a software you use but you are not entirely familiar with. The null return might pass through multiple call layers until it reaches your code which makes debugging that kind of code a journey through layers of foreign and undiscovered code – sometimes this can be fun but almost never what you want to do when in a hurry:
class BrokenInnerClass {
public function innerMethod() {
// …
if ($error) {
return null;
}
// …
}
}
class DispatchingClass {
public function dispatchingMethod() {
return $this->brokenInnerClass->innerMethod();
}
}
class MyUsingClass {
public function myBeautifulMethod() {
$value = $this->dispatchingClass->dispatchingMethod();
$value->getSomeProperty(); // Fatal Error
}
}Usually there are even more levels of indirection, of course. We live in the age of frameworks after all.
If a value could not be found do not return null but throw an exception – there are even built in exceptions for such cases like the OutOfBoundsException, for example.
In the callstack I can see immediately where something fails. In the optimal case the exception message even adds meaning and gives some hints of what I have to fix.
Using null can be valid inside of value objects and sometimes you just want to show nothing is there. In most cases null should be either replaced by throwing an exception or providing a null object which fulfills the API but does nothing. Those null objects are trivial and fast to develop. The return on investment will be huge due to saved debugging hours.
Stay up to date with regular new technological insights by subscribing to our newsletter. We will send you articles to improve your developments skills.
Richard on Tue, 03 May 2016 11:19:26 +0200
In your first example. It sounds like $logger is required. It should be passed as part of the constructor so the coder never forgets it.
Link to commentThis isn't a problem with null, it's a problem with allowing the user to make a mistake.
Sjoerd Maessen on Tue, 03 May 2016 11:22:54 +0200
Good article, I agree with the fact using null can make it harder to debug in most cases.
Link to commentA small addition about the NullLogger. I think the code could be improved by making the Logger instance non-optional in the constructor. If you always require an instance of Logger you remove the dependency to NullLogger and it keeps the service class cleaner. I believe it's not the responsibility of the "SomeService" class to make an instance of NullLogger and fill the property. But it's the responsibility of the callee to provide an instance that will be used.
Andreas Czakaj on Tue, 03 May 2016 13:37:54 +0200
Kore, thanks for your interesting post.
Link to commentNull object is indeed a pattern that is used too rarely.
In addition, I'd like to hint at yet another common pattern that is frequently used in functional programming, i.e. Option Type, cf. https://en.wikipedia.org/wiki/Option_type
PHP seems to have no native support for it (yet) but it should be feasible to code something similar.
Cheers,
Andreas
Saji Nediyanchath on Wed, 04 May 2016 04:03:09 +0200
Feels like a pretty good idea and an easily implementable one as well. Thanks for sharing it.
Link to commentRomanko on Wed, 04 May 2016 07:59:07 +0200
Agree with the first example. But totally disagree with the last one as it is too ambitious.
Link to commentConsider this:
# All PHP variables by default are NULLs until initialised. Because it is a default behaviour it is better to remember about it and validate / verify your data or properties your class has before making calls or whatever.
# A number of built-in PHP functions may return NULL and so it may bubble up into your code. See above.
# In many cases NULL as a return value literally means "no data" and so what's a point in throwing exceptions?
# Speaking of exceptions: method1 calls method2 to get data. Method2 does not have data and it throws an exception. However, for the method1 it may be perfectly valid not to receive any data at this stage. However, its intention to get data and continue is badly interrupted with exception thrown in method2.
Unchecked exceptions must be used wisely. It is all good and fun when you can clearly see a stack trace and in perfectly designed application it is a "must" however, your app must do what it is intended to do, you don't develop with single intention in mind to have a clear stack trace.
Michael Butler on Wed, 04 May 2016 13:41:23 +0200
A few are saying you should make the logger required in the constructor... Sure we could do that, but then *everywhere* that calls it would need to be updated. That may be difficult or impossible if it's not your code.
Link to commentBy defaulting to a nullLogger we are doing it safe and with a far smaller blast radius of change.
Henning Kvinnesland on Tue, 10 May 2016 10:41:08 +0200
Masking fatal errors like seems like a terrible idea to me. Instead of a clear error-message, the new developer has a fully working application which for "some reason", does not cache or does not log in production. You have just added another layer of confusion. If the application only fails in production, what you are missing is a staging area where you can properly test the application before deployment.
Link to commentPawelzny on Thu, 23 Feb 2017 19:42:35 +0100
Null reference has special meaning: "no value".
Link to commentThis is very useful information. Interrupting application flow with exceptions which are not exceptional at all gives no advantage for me.
Sofi on Thu, 23 Feb 2017 23:08:50 +0100
This entire section is wrongly formulated. Like some have already said (you might wanna comment voting here, I'd vote few up)masking initialization issues with default "do nothing implementations" is the worst of all practices as you end up having code that seems to be running all right but it is not fulfilling its purpose. Why, pardon my words but who the hell knows... this is cost.
Link to commentOf course null should not be used to mask or hide away error cases, but this is just point of bad design and one that needs to improve his/her skills.
The solution in overall null fame is educating people how to use it properly and not, faking things and saying null is bad practice.