From the get-go, let's agree that static
and the Singleton pattern are quite pointless in PHP. By design, PHP is a stateless language. The state of an object is not retained in between requests. Hence, static
is often referred to as mere globals in OO-drag.
I would like to stress that, in PHP, Singletons are as pointless as a broken pencil.
Be that as at it may, a Singleton
trait is an interesting thought. It's a good way to visualize, test and eventually understand inheritance precedence, when traits are involved. And it can also show how magic constants and certain keywords behave unexpectedly.
But it's nothing more than that: an academic example, an exercise and... an example of bad practices.
Don't abuse traits
What you do is very bad practice indeed. You take a concept, meant to do one thing, and abuse it to do something entirely different. You're using a trait, not to get around the (alleged) limitations of single inheritance to mimic multiple inheritance.
This is not what a trait is for
Ask yourself: "What is the point of a trait?", why were the introduced? They were created to solve (IMHO imagined) problems, posed by single inheritance (which actually is a good thing). Traits are an attempt to offer the benefits of multiple inheritance without all of the headaches. A trait is a unit of code that extends the functionality of a class. You are using a trait to dictate the behaviour and usage of a class.
Consider the syntax:
class Bar extends Foo{}
Ok, a class that builds on a fully-fledged (stand-alone) class. The extends
keyword makes sense.
Now:
class Something implements Iterator{}
A class, that implements an interface. The interface dictates (part of) the behaviour and can be used to enforce a contract. A class that implements an interface has to conform. Just like JavaScript is an implementation of the ECMAScript standard, and compilers ship with an implementation of the C/C++ standard, so must this class be an implementation of the interface's contract.
Again, the keyword implements
covers the load.
Look at the syntax for traits, though:
Class Something extends Base implements Iterator
{
use DBGetters;
}
The use
statement is part of the class, it does not belong to the class definition. A trait, then, has no business enforcing a contract in the way an (abstract) parent class does. Nor should it dictate the behaviour of a class, like an interface does.
Everything you have (Classes, interfaces and traits) are meant to address a specific issue you might encounter.
A trait adds functionality. It does not change the way a class functions.
Still, some core-review:
The issues up front:
- try to adhere to the coding standards as much as possibel
- always specify an access modifier, don't rely on the implicit
public
. Someone might come across some code, and treat the lack of an access modifier as an error, and make it protected
.
- static methods begin with an UpperCase
- constructors take arguments
- To ensure a method is not overwritten in child classes, use the
final
keyword
- Initializing variables is a good habit. Though not required in PHP (default value is null), it's a habit that makes the transition to other languages easier/smoother. Initialize to
null
explicitly
Let's put all this to practice when writing your trait.
trait Singleton
{
final public static function GetInstance(array $args = null)
{
static $instance = null;
if ( is_null( $instance ) ) {
$instance = new static($args);
}
return $instance;
}
}
That, to my eye, looks a lot cleaner, but there are problems. With this code, your trait is implicitly enforcing a contract: the __construct
methods will now be passed either null
or an array, and so all constructors must be defined to conform to this contract.
The static variable that retains the instance is a static variable is also local to the method's scope. I know why you do this, but there are implications, and alternatives. One such implication is (owing to PHP's memory management system, which relies on simple ref-counts), that the instance will not be eligible for garbage collection (most static's aren't, for the same reason).
However, a regular singleton class, in PHP does allow for its ref-count to be set to 0, and can be garbage collected (in a hacky way):
class GCSingleton
{
private static $Instance = null;
//private constructor and such
final public static function GetInstance()
{
if (self::$Instance === null)//or static::$Instance
self::$Instance = new static;
return self::$Instance;
}
public function selfDestruct()
{
self::$Instance = null;
return null;
}
}
$singleton = GCSingleton::GetInstance();
$singleton = $singleton->selfDestruct();
This only works, though, if no other variable references the instance in question, but no matter.
Of course, I completely understand why you did not create a property to which you assign the instance. A static instance is, after all, shared, so using a static property makes inheritance impossible, right?
Not quite: enter inheritance precedence. You can override a static property in a child class, with the same (or less) restrictions:
trait Test
{
private static $Inst = null;
final public static function GetInstance()
{
if (static::$Inst === null)
static::$Inst = new static;
return static::$Inst;
}
}
class Foo
{
use Test;
public function __construct()
{
echo 'Created new instance of ', __CLASS__, PHP_EOL;
}
}
class Bar extends Foo
{
protected static $Inst = null;
public function __construct()
{
echo 'Created instance of ', __CLASS__, PHP_EOL;
}
}
$foo = Foo::GetInstance();//echoes Created instance of Foo
$bar = Bar::GetInstance();//echoes Created instance of Bar
If, however, you change the use of the keyword static
to self
, you loose the power of late static bindings, and Bar::GetInstance
returns the instance of Foo
, assigned to the private property of the trait.
Ok, now that's some under-the-hood info of how the properties are being resolved, and how the resources are managed. Let's move on to some actual code-review:
Omissions
The whole point of singletons is that they are there to make sure you never create more than one single instance of a given class. You are aware of the __clone
magic method, but you're missing the __sleep
and __wakeup
, or even the __toString
methods.
The best way to address this issue, IMO, is to define all of them (as final
) in the trait, and instead of letting them fail silently, have them throw exceptions. The moment someone writes something like:
$copy = clone $singleton;
An exception should be thrown. You're just assigning null to $copy
, which won't produce errors until someone uses this $copy
variable as an object. This can lead to hours, wasted on debugging code, looking for that statement that reassigns $copy
, that just isn't there. Instead, write something like:
final public function __clone()
{
throw new LogicException(
sprintf(
'Error: cloning of %s instances is not alllowed',
get_class($this)
)
);
}
Do this for all magic methods.