Favour Composition over Inheritance
Wednesday, June 4th, 2008
I was just reading through the current issue of php|architect (April 2008) and I noticed a particular piece of code that irked me to the point that I need to write about it. If you’re interested, the article is Exceptional Error Handling. Don’t get me wrong, it’s a well written and informative article, however, one of the listings (Listing 9) has some really hacky code in it. The hack is only in the constructor, but the flaw is in the design. A flaw that can easily be overcome with a piece of sage advice from the Gang of Four: “Favour object composition over class inheritance” [GoF20]
The intention of the code is to have a singleton class that extends PHP’s ArrayObject class. Here’s the code snippet:
class LogService extends ArrayObject
{
private static $instance;
public function __construct(Array $loggers = array())
{
// examine backtrace to see how the constructor was called
$backtrace = debug_backtrace();
// throw exception if we were not called from the
// getInstance() method of this class
if ($backtrace[1]['class'] !- __CLASS__ ||
$backtrace[1]['function'] != 'getInstance'){
throw new SingletonException('...');
}
parent::__construct($loggers);
}
public function getInstance(Array $loggers = array())
{
if (!self::$instance){
self::$instance = new LogService($loggers);
}
return self::$instance;
}
...
}
NB: i left out the rest of the class and the content of
the exception message because it's not really relevant
to the point at hand.
Can you spot the problem here?
The problem is that this breaks one of the main rules of the singleton pattern — that new instance of the class cannot be created from outside of the class — and then attempts to reconcile it by checking the backtrace and throwing an exception. Since the constructor of the ArrayObject class is defined as public, any child classes of ArrayObject must also have their constructor defined as public. This is because access to a method of a child class cannot be more restrictive than that method in the parent class. The final solution ended up being a hacky mimic of the singleton pattern.
Ockham’s razor (or at least a paraphrased version): “the simplest solution is the best”
Checking the backtrace and throwing an exception is not the simplest solution. The simplest solution is to favour composition over inheritance.
Let’s look a little deeper at the ArrayObject class
You’ll see that the ArrayObject implements 3 interfaces: IteratorAggregate, ArrayAccess, and Countable. These interfaces are what allow php to treat instances of the ArrayObject as native array, i.e. using it in a foreach loop, accessing elements of the ArrayObject, finding out how many elements are in the ArrayObject, etc.
Let’s see how we can use composition and interfaces to write an equivalent singleton class without the need for the hacky constructor
class LogService implements IteratorAggregate, ArrayAccess, Countable
{
// the only available instance of this class
private static $_instance;
// an instance of ArrayObject. using composition rather than
// inheritance
private $_array;
// private constructor as an optimal solution for the singleton
// pattern
private function __construct(Array $loggers = array())
{
$this->_array = new ArrayObject($loggers);
}
// method required by the IteratorAggregate interface
public function getIterator()
{
return $this->_array->getIterator();
}
// method require by the ArrayAccess interface
public function offsetExists($index)
{
return $this->_array->offsetExists($index);
}
// method require by the ArrayAccess interface
public function offsetGet($index)
{
return $this->_array->offsetGet($index);
}
// method require by the ArrayAccess interface
public function offsetSet($index, $newval)
{
return $this->_array->offsetSet($index, $newval);
}
// method require by the ArrayAccess interface
public function offsetUnset($index)
{
return $this->_array->offsetUnset($index);
}
// method require by the Countable interface
public function offsetCount()
{
return $this->_array->count();
}
public function getInstance(Array $loggers = array())
{
if (!self::$_instance){
self::$_instance = new LogService($loggers);
}
return self::$_instance;
}
....
}
In my opinion this is a much more elegant solution, even if it does require a little bit more coding (and very trivial code at that). There’s no way to call the constructor outside of the class and thus no need to manage the possibility of an exception thrown by the constructor. It serves the same purpose as the original code (you could even implement the php’s magic method __call() in the LogService class to essentially proxy method calls to the instance of the ArrayObject) and is more inline with the intention of the Singleton pattern.
What do you think? Which solution would you prefer? What do you think is more important: good design practices or less lines of code?
2 Comments 
