2

In this example I have classA and classB that I am using with pimple container.

They both have a dependency on each other. However when setting this up with pimple DIC, the below code causes an infinite loop...

There must be a way of doing this in pimple but I can't see it in the docs... Any ideas how to prevent the infinite loop?

// PIMPLE CONTAINER
use Pimple\Container;
$container = new Container();

use Classes\ClassA;
$container['ClassA'] = function ($c) {
  return new ClassA($c['ClassB']);
};

use Classes\ClassB;
$container['ClassB'] = function ($c) {
  return new ClassB($c['ClassA']);
};
yivi
  • 42,438
  • 18
  • 116
  • 138
jon
  • 1,429
  • 1
  • 23
  • 40

3 Answers3

1

Strictly speaking, your problem is not really pimple related.

You simply have a circular constructor dependency, there is no way to fix those. A needs B needs A needs B...

The thing is, usually, two classes that depend on each other do not make much sense. But in the rare case where that do happen, one of the two should be a lighter dependency, where you inject the object through a setter or some mechanism like that after instantiation.

By passing the container around and instantiating inside ClassB, you are hiding your dependencies, which beats the purpose of having a dependency injection container. Now ClassB depends on Container, not on ClassA, which is what you wanted in the first place.

Instead of doing that, just add a method setA(Class A $a) to your ClassB.

Then, you inject your dependency by calling $b->setA($container['ClassA']); when you actually need it, not on the DI container. As Adam points out, you can even do the setter injection in your container by extending your services and using the setters on the service definition.

But again, to reiterate, your main problem is to have a circular dependency. Re-think that. It's very likely a sign your design could do with some improvements.

Community
  • 1
  • 1
yivi
  • 42,438
  • 18
  • 116
  • 138
  • It might not be a Pimple problem, but it's solvable with Pimple. See my answer. I think the combination of yours and mine is useful, so I'll cross ref to your one in my one I think. – Adam Cameron Mar 08 '17 at 07:14
  • @Adam, Yup, I was going to suggest calling the setter in the service definition, but I still think that his main problem is having a circular dep. IMO (much) more often than not, that is a sign that something else is amiss with the design. Still, I like your answer. :) – yivi Mar 08 '17 at 07:17
  • Yep. I've re-emphasised this in my answer too. Good team work! – Adam Cameron Mar 08 '17 at 09:59
1

@yivi makes a valid observation in their answer about the validity of circular references in the first place. So I really think you ought to assess your design here. You might be treating the symptom, not the underlying problem. It's impossible for us to comment on this though given your generic sample code (which is good code for the issue at hand, that said). Maybe raise a new question about your design if you think it's worth while? Either here or on Code Review, maybe?

If you are in control of the design of ClassA and ClassB, then the prescribed approach to this sort of thing is to use setter injection, not constructor injection.

This should work:

// PIMPLE CONTAINER
use Pimple\Container;
$container = new Container();

use Classes\ClassA;
$container['ClassA'] = function ($c) {
  return new ClassA();
};

use Classes\ClassB;
$container['ClassB'] = function ($c) {
  return new ClassB();
};


$container->extend('ClassA', function ($instanceOfA, $container) {
    $instanceOfA->setB($container['ClassB']);

    return $instanceOfA;
});

$container->extend('ClassB', function ($instanceOfB, $container) {
    $instanceOfB->setA($container['ClassA']);

    return $instanceOfB;
});

(untested, might contain typos, but that's the general gist)

Note how the original constructors do not take the dependency any more, the insertion of that is left to a specific setter. This allows you to create the services in the container, then use extend to then call the relevant setter to insert the dependency after this.

This is in the docs: Modifying Services after Definition.

Community
  • 1
  • 1
Adam Cameron
  • 29,677
  • 4
  • 37
  • 78
  • Thanks very much for both of your responses @Yivi and Adam Cameron. You both allude to the fact that this circular constructor dependency problem is more down to bad design, which is possible. The scenario I have is - I have a Query class and Validation class. The Query class has many methods that require data to be validated through the validation class. One of the validation methods requires a method in the Query Class. Therefore is there a better way for me to structure the code?.. or is the above scenario ok if I tackle it using one of your solutions. Thanks again for your responses. – jon Mar 08 '17 at 19:11
  • @jon like I suggested: raise a separate question, I reckon. This bit is not really related to *this* question, plus it'll give you more space to manoeuvre. Also you can broaden its context away from Pimple, DI, and even PHP as it's a general OO design question. I recommend you implement stub classes with the actual method names and usages to better contextualise the situations: names and usage in this case are important to design decisions. It might be the case your design is correct, btw. It's hard to tell. – Adam Cameron Mar 09 '17 at 05:42
  • Seems that this solution does not work. `extend('ClassA'` is called straight after ClassA fist usage. Then, `ClassB constructor` is called followed by `ClassB extend` that needs ClassA. Resulting in a loop. Better fix classes design ! – Jerem Jan 30 '21 at 22:43
-1
// PIMPLE CONTAINER
use Pimple\Container;
$container = new Container();

use Classes\ClassA;
$container['ClassA'] = function ($c) {
  return new ClassA($c['ClassB']);
};

use Classes\ClassB;
$container['ClassB'] = function ($c) {
  return new ClassB($c);
};

Instead of instantiating ClassA from classB, pass the container to ClassB. Then when you need ClassA within ClassB, you can use the container you have passed to it to initiate / get instance ot ClassA.

jon
  • 1,429
  • 1
  • 23
  • 40