Storage class, dependency injection and singletons

Question

In re-writing my PHP framework vervPHP, I've created the following class:

class storage {
 private static $instance;
 public $db = null; // Holds a database connection object
 public $user = null; // Holds user related information for custom auth
 public $data = array(); // Array of data for storage
 private function __construct(){
 // Create the default array in $this->data....
 }
 public static function singleton() {
 if (!isset(self::$instance)) {
 $className = __CLASS__;
 self::$instance = new $className;
 }
 return self::$instance;
 }
 // Getters, Setters and other stuff follows...
}

When my framework starts up, it looks like this:

index.php:

// Kick it off!
verv\init();

framework/verv/verv.php:

function init() {
 $verv = storage::singleton();
 loadConfig($verv);
 loadRequest($verv);
 connectDB($verv);
 authenticateUser($verv);
 loadModule($verv);
 loadTemplate($verv);
 renderPage($verv);
}

$verv is passed by reference - so any changes are saved as it moves around.

My question is, do I need the singleton portion? And basically, am I doing it "right"? The code in question works (albeit it needs a little tidying) but I want to make sure that my storage class is as robust and efficient as possible.

• mseancole: I think you've misunderstood the concept of singleton here. From my understanding a singleton class should not be instantiated with the constructor and therefore should not have one. It should never even be used outside of a static scope. Now, as to your init() function. This looks like it should be the constructor of some class, I would consider setting $verv as a class property so that you don't have to pass that as a parameter each time. It will serve the same purpose. I have no real experience with the Singleton pattern, however, from everything I've read this explanation seems right.
• Amy Blankenship: You can mitigate the bad effects of using Singletons by passing the reference out to the "leaf" code through dependency injection and only have logic close to the core/base know about the Singleton. This makes it easier to get rid of the offending object later when you realize how hard it is to maintain a large project that uses it. However, it seems you have an intuition already, so why not avoid the pitfalls now?

Answer 1

Read this post before using the singleton pattern: https://stackoverflow.com/a/4596323/1908639

You might find that it increases your initial development speed; however, the downsides tend to increase with the size/complexity of the project. I have personally maintained code that used singletons, and I found it difficult to test/reuse.

If you think there are any major advantages to the singleton approach, I would be interested in hearing them. I have not been able to find any beneficial use-cases in PHP web development, but I am definitely interested in hearing about them, if they exist.

Also, passing objects by reference should not be necessary in PHP 5 (source):

In PHP 5 there is a new Object Model. PHP's handling of objects has been completely rewritten, allowing for better performance and more features. In previous versions of PHP, objects were handled like primitive types (for instance integers and strings). The drawback of this method was that semantically the whole object was copied when a variable was assigned, or passed as a parameter to a method. In the new approach, objects are referenced by handle, and not by value (one can think of a handle as an object's identifier).

Answer 2

I think you are going to the wrong direction with this kind of aproach. First you are creating a Singleton while propably you don't need to use it. Second you are mixing things in one God object which is also a bad design in an object-oriented environment. And what is this init() function? Just laying around in a namespace alone in the dark? Bad design. Most beginner PHP programmers are don't know that passing by reference things are can really hurt performance and BTW objects are passed by not reference but an internal handler which will give you the possibility to modify (not unset or overwrite) that object.

• EvilChookie: I think you may be getting confused by passing by reference (good!) versus returning by reference. Passing by reference is the whole point of Dependancy Injection, and is considered A Good Thing. Also, please check out codereview.stackexchange.com/questions/10060/… to see what it looked like before, and what feedback I recieved regarding it.
• Peter Kiss: Passing by reference and dependency injection are two different things. Passing by reference is a type of argument handling in functions and methods provided by PHP, dependency injection is a design pattern what can be used in any programming language. When you say: public function __construct(ISameInterface $obj) {/* ... */} you are not passing by reference anything but you are using constructor injection.

Answer 3

Because you are passing things in via dependency injection, it does render the singleton a bit pointless in the code you have shown. Normally, it is useful in that you can call the storage::singleton() method from anywhere and know that you will have the exact same object. However, this causes a problem because unit testing becomes difficult as you can't decouple the function from the singleton, so your code as it stands is far easier to test.

You do however have the advantage with the singleton that other people using your framework can't wreck it by ignoring the dependency injection and just making a new instance of the storage class somewhere, so actually I think this is a good approach.

I'm not quite sure why you have the page construction process contained within init() as you could really just have those raw function calls on the main index.php page. You appear to be already using a router pattern of some sort, so presumably index.php acts a single entry point anyway.

• Amy Blankenship: Maybe those people using the framework have a good reason to have two instances, and by forcing only one instance you can render your code useless for them.