13

I have used Dice PHP DI container for quite a while and it seems the best in terms of simplicity of injecting dependencies.

From Dice Documentation:

class A {
    public $b;
    
    public function __construct(B $b) {
        $this->b = $b;
    }
}

class B {
    
}

$dice = new \Dice\Dice;    
$a = $dice->create('A');
var_dump($a->b); //B object

However, when you have to use objects that are directly dependent on each other, the finall result is server error, because of the infinite loop.

Example:

class A {
    public $b;

    public function __construct(B $b) {
        $this->b = $b;
    }
}

class B {
    public $a;

    public function __construct(A $a) {
        $this->a = $a;
    }
}

Author of Dice says that there is no way to construct an object from the A or B classes. As:

  • An 'A' object requires a 'B' object to exist before it can be created
  • But a 'B' object requires an 'A' object to exist before it can be created

Author says, that this limitation concerns all DI containers!


Question:

What would be the best solution for overcoming this problem nicely without changing initial code? Could anyone provide an example of using other DI containers, when it would be possible to run exampled code without bulky workarounds?

Community
  • 1
  • 1
Ilia Ross
  • 13,086
  • 11
  • 53
  • 88
  • 3
    Take a moment to forget about the container entirely. Can you create either of these objects yourself? You 've created a circular dependency, it's not the container's fault that it cannot resolve it. – Jon Jun 06 '14 at 08:11
  • @Jon It's not the containers fault, yes! But if I remove one of the dependencies from one of the objects, then they work just fine. The problem that I need to use A from B, and B at the same time relies on insignificant usage of A. I can't solve this problem *nicely*. (I'm not looking just for a solution). What you could do here really? – Ilia Ross Jun 06 '14 at 08:21
  • Splitting the significant and insignificant functionality of A into two separate entities sounds like a plan. Perhaps the current version of A has too many responsibilities? – Jon Jun 06 '14 at 09:10
  • @Jon If I do so, then my project structure will start loosing its sense. You can see basic idea of what responsibilities it has. https://github.com/TomBZombie/Dice/issues/7#issue-35102964 – Ilia Ross Jun 06 '14 at 09:12
  • I can see the class names but that doesn't tell much. Why does Session need Language and Language need Session? – Jon Jun 06 '14 at 09:15
  • For example, because *Language* class needs to use `$this->session->set()` / `$this->session->get()` functions and *Session* class at the same time has a function `session_messages()` which uses `translate()` function that is part of *Language* class. Anything wrong with that? – Ilia Ross Jun 06 '14 at 09:22
  • Lots of things are wrong with that. Why does Language include functionality to not only translate messages but also to persist itself? Why does Session know or care about translations instead of being simply a dumb data store? In both cases the answer is probably because that allows for you to write code more conveniently. But it's a bad design because it violates the SRP. In your defense, making good designs that are also easy to use is in general a very difficult task. – Jon Jun 06 '14 at 09:59
  • @Jon `Why does Language include functionality to not only translate messages` - because it's multilingual project (pain in the back with this multilingual stuff) and it returns messages based on session (user) language, and this is why *Language* class needs to do stuff with *Session* class, while *Session* class needs to return `session_messages()` based on user language. What would you do/recommend to accomplish tasks like I described and comply with *SRP* at the same time? – Ilia Ross Jun 06 '14 at 10:16
  • Language should not have a dependency on Session; it should have a dependency (or possibly even no dependency, it could be configured on the fly) on the language code that corresponds to what you want it to do. Where you get that code from is none of Language's business. And as I said before, Session should be a dumb data store. – Jon Jun 06 '14 at 10:24

2 Answers2

10

As mentioned on your post on the Dice github ( https://github.com/TomBZombie/Dice/issues/7 ), the only way to resolve without removing the circular dependency is to refactor one of the classes to use setter injection:

class A {
    public $b;

    public function __construct(B $b) {
        $this->b = $b;
    }
}


class B {
    public $a;

    public function setA(A $a) {
        $this->a = $a;
    }
}

This allows the objects to be constructed:

$b = new B();
$a = new A($b);
$b->setA($a);

With the original code:

class A {
    public $b;

    public function __construct(B $b) {
        $this->b = $b;
    }
}

class B {
    public $a;

    public function __construct(A $a) {
        $this->a = $a;
    }
}

You cannot construct it and run into the same problem as the container:

$b = new B(new A(new B(new A(new B(.............))))

The problem with having a container work around this issue using a hack such as ReflectionClass::newInstanceWithoutConstructor is that your objects are now dependent on creation logic which uses this method. You essentially couple the code to the container which is a poor design as your code is now no longer portable and cannot be used without the container to perform the object construction.

Tom B
  • 2,735
  • 2
  • 24
  • 30
  • Thanks, Tom! You wrote `You essentially couple the code to the container which is a poor design as your code is now no longer portable` - what would be the right way of doing it? – Ilia Ross Jun 06 '14 at 09:52
  • 1
    I have updated Dice to work around the issue, however it's still poor design but it will stop the infinite loop. – Tom B Jun 06 '14 at 10:41
  • I will fix my design! Is it bad as it breaks *SRP* (Single Responsibility Principle)? – Ilia Ross Jun 06 '14 at 12:51
  • 3
    The cyclic reference itself implies that both objects share a responsibility, it's likely that one (or both) of the classes are doing too much (looking at the amount of dependencies they have, this is almost certainly the case, see http://misko.hevery.com/code-reviewers-guide/flaw-class-does-too-much/ ). If they really are that reliant on each other, they are a single responsibility. – Tom B Jun 06 '14 at 13:18
  • Tom, thanks a lot for your help and recommendations. I have finally changed my patterns completely, so my classes now are not *doing to much* and I have avoided a lot of recursions as well. My application gained in speed. Many thanks. *P.S. I hope one day you will release **Dice** with comments and `GNU style` formatted (as I emailed you once - http://codepad.viper-7.com/yqJ8cl ). Best regards.* – Ilia Ross Jun 14 '14 at 16:14
7

You have a circular dependency, which is very hard to solve. The first thing to do is to try to get rid of this circular dependency by refactoring your classes and how they interact.

If you really can't manage to do it, there are solutions. I'll copy-paste my answer from Self-referencing models cause Maximum function nesting level of x in Laravel 4:

  • Setter injection

Rather than injecting a dependency in the constructor, you can have it injected in a setter, which would be called after the object is constructed. In pseudo-code, that would look like that:

$userRepo = new UserRepository();
$cartRepo = new CartRepository($userRepo);
$userRepo->setCartRepo($userRepo);
  • Lazy injection

I don't know if Dice does support lazy injection, but that's also a solution: the container will inject a proxy object instead of the actual dependency. That proxy-object will load the dependency only when it is accessed, thus removing the need to build the dependency when the constructor is called.

Here is an explanation on how lazy injection works if you are interested: http://php-di.org/doc/lazy-injection.html

Community
  • 1
  • 1
Matthieu Napoli
  • 48,448
  • 45
  • 173
  • 261