3

I have a main bootstrap class (singleton1 in the example bellow) that instantiates some singleton classes. In those singleton classes, I need to keep a reference to the app main class for easy and quick reference to it, but doing so gives me a:

Fatal error: Maximum function nesting level of '100' reached, aborting

Here is the exmaple code:

<?php

class Singleton1 {

    private static $instance;


    private function __construct() {

        Singleton2::instance();

    }

    public static function instance () {

        if( !self::$instance ) {
            $class = __CLASS__;
            self::$instance = new $class;
        }

        return self::$instance;
    }

    public function test () {
        echo 'Good.';
    }

    private function __clone() { }

    private function __wakeup() { }

}

class Singleton2 {

    private static $instance;

    private $singleton1;

    private function __construct() {
        $this->singleton1 = Singleton1::instance();
    }

    public static function instance () {

        if( !self::$instance ) {
            $class = __CLASS__;
            self::$instance = new $class;
        }

        return self::$instance;
    }

    public function test () {
        $this->singleton1->test();
    }

    private function __clone() { }

    private function __wakeup() { }

}

Singleton1::instance();

Singleton2::instance()->test();

I can't find any logical explanation to the issue ...

Thanks.

  • 2
    To begin with , this is a pretty horrible application design. You really should avoid using Singletons in your code and favor Dependency Injection instead. – tereško Oct 04 '11 at 07:58
  • Watch [this lecture](http://www.youtube.com/watch?v=-FRm3VPhseI) and the rest of Clean Code Talks. Maybe you will understand. – tereško Oct 04 '11 at 08:04
  • Dependency Injection is a fancy term, simply meaning: pass objects to the constructor, or via a setter method. (Usually contrasting retrieving objects via some type of registry or singleton pattern, as you are doing now). – Decent Dabbler Oct 04 '11 at 08:10
  • @fireeyedboy , no, it is not. If you do it that way , then you are not using Dependency Injection .. and you are calling it DI just because it is a fancy term. – tereško Oct 04 '11 at 10:21
  • @tereško: At the root of it, it *is*. You are injecting dependencies (generally either via the constructor or via a setter). That DI is generally also implemented by additionally using some container and/or factory to create instances of the objects and hooking the dependencies together and what have you, doesn't make DI in itself any more than simply injecting objects with what they are depending on. – Decent Dabbler Oct 04 '11 at 10:33
  • @fireeyedboy , but none of the dependencies is global by nature, in proper DI. You are able to replace any dependency if you want ( usually they all are in the bootstrap file ). That is the whole point. – tereško Oct 04 '11 at 11:18
  • @tereško: I agree with you of course. I guess the reason I sometimes (in this case) object to this use of jargon, is that it probably confuses people more than that it clarifies. If I say: try to pass the object to the constructor (or a setter), I think it might be a little more closer to home, and more easily leads to understanding the implications and/or usefulness of such a concept (because of small incrementally learned steps), than by saying: use DI and possibly intimidate somebody with a concept that is totally foreign to them. (But perhaps that's just how my brain works ;-)). – Decent Dabbler Oct 04 '11 at 11:33
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/3996/discussion-between-teresko-and-fireeyedboy) – tereško Oct 04 '11 at 11:48

3 Answers3

4

Your code has the following loop:

Singleton2::__construct()
Singleton1::instance()
Singleton1::__construct()
Singleton2::instance()
Singleton2::__construct()
Sjoerd
  • 74,049
  • 16
  • 131
  • 175
  • Yes, I think I just got it, when I do $this->singleton1 = Singleton1::instance(); in Singleton2 constructor, Singleton1 has not finished to instantiate, so the instance is not yet available in the static $instance var, thus it tries to instantiate it again, and again... – Laurent Dinclaux Oct 04 '11 at 07:56
2

You can remove Singleton2::instance() line from Singleton1 constructor to see "Good" message.

Vladimir Fesko
  • 222
  • 2
  • 3
  • Of course. But there is a particular reason that the Singleton2 should be instantiated by the main class. I think I'll end up using an init method to get ran when I instantiate the main class, the first time. – Laurent Dinclaux Oct 04 '11 at 08:04
  • Yep, just don't instantiate Singleton2 in Singleton1's constructor since Singleton1 instance is not created by that call. – Vladimir Fesko Oct 04 '11 at 08:12
1

You should just not use Singletons. They're Bad Practice(tm), as they introduce global state, which makes your Code much harder to test.

One could rewrite your example by using Inversion Of Control this way:

<?php

// A simple Singleton for the Application
function Application()
{
     static $instance = null;

     if (null === $instance) {
         $instance = new Application;
     }
     return $instance;
}

class Application
{
     protected $someService;

     function __construct()
     {
         $this->someService = new SomeService($this);
     }

     function test()
     {
         echo "Good.";
     }
}

class SomeService
{
     protected $application;

     function __construct(Application $app)
     {
         $this->application = $app;
     }

     function test()
     {
         $this->application->test();
     }
}

Application is your Main Application's class and SomeService is your Singleton2.

No need for private constructors, etc. here and SomeService gets only instantiated once, with your App (considered you only instantiate your App once).

Repeat this with other Services you want to add to your App instance.

Some good articles on this, which are worth reading:

  1. http://www.potstuck.com/2009/01/08/php-dependency-injection/
  2. http://fabien.potencier.org/article/11/what-is-dependency-injection
chh
  • 593
  • 3
  • 8
  • 1
    While that is slightly better, you now have a circular reference which is still quite bad. – igorw Oct 04 '11 at 08:07
  • I used to do such things in the past but thought that I have to use singleton for classes that are intended to be instantiated only once. Isn't that Good Practice ? – Laurent Dinclaux Oct 04 '11 at 08:12
  • You will create your application instance only once, but you could make the Application a singleton, so the service instance gets also only created once. The main point here is, that you should at least inject the application instance into the service via a Setter/Constructor to avoid the call to `Application::getInstance()`, which causes the recursion. – chh Oct 04 '11 at 08:53
  • @igorw That shouldn't be a Deal-Braker? PHP should free all memory used by the app on request end. Though I also think it should be avoided. – chh Oct 04 '11 at 08:59