DI Configuration: Circural references

I like the “stash” solution.

Do you think that would be faster and more memory effective? (I don't know, just asking).

Circular reference is often sign of design error. But sometimes there is no simple way to avoid it. I proposed solution before. Look here https://forum.nette.org/…v-containeru#… Basically it is only variant of your stash solution. What I think is that this behaviour should not be default but activated explicitly. Because you will be passing only partially initialized service. If you did not do any real work in constructor which you should not it does not matter but I think you should activate this feature only if you know what you are doing.

Last edited by mystik (2014-05-28 21:15)

I agree solution proposed here is better than mine. It is much simpler with equivalent outcome. I just think this behaviour should be explicitly enabled not implicit.

The problem could occur if partially initialized service is used during creation of service graph. If some service try to use partially initialized service which it just received in constructor or by setup it can result in unexpected behaviour. I think it would be better throw circular reference exception in this case unless it was explicitly stated that service can be passed partially initialized. It is not necessary but think it can prevent some hard to debug problems.

Yes, but if I understand your solution right then partially initialized services from stah will be passed to their dependencies.

In your original example when service apiIo is created it gets partially initialized apiClinet passed in it's constuctor.

In this case it is ok, but imagine situation where implementation of apiIo constructor will expect that apiClient is fully initialized a use it. If it throws exception then it will fail immediately and you can find solution.

But what if will be used something like this:

class ServiceA() {

  private $serviceB;
  private $serviceC;

  public function setServiceB($service) {
    $this->serviceB = $service;
  }

  public function setServiceC($service) {
    $this->serviceC = $service;
  }

  public function createSomethingWithServiceC() {
    return new Something($this->serviceC);
  }

class ServiceB() {

  private $serviceA;
  private $something;

  public function __construct($serviceA) {
    $this->serviceA = $serviceA;
    $this->something = $serviceA->createSomethingWithServiceC();
  }

Then can this sequence of call occur:

• $serviceA = newServiceA()
• $container->stash['serviceA'] = $serviceA
• when trying to call setup for setServiceB creation of ServiceB is triggered
• $serviceB = newServiceB($container->stash['serviceA'])
• serviceB is creted but in it's $this->something is wrongly initialized Something because null was passed to it's constuctor, setServiceC of ServiceA was not called yet
• we return to setup of ServiceA
• $serviceA->setServiceB($serviceB)
• $serviceA->setServiceC($serviceC)

This problem can be very hard to debug buecause when you try to test it from outside of container when you call createSomething() you get corect result, serviceC seems to be correctly set in serviceA yet we still have Something in serviceB with null passed to it's constructor.

Or did I somehow misunderstood how your solution should work?

Last edited by mystik (2014-05-29 08:12)

Well I agree with all your points :-)

• problem occurs only during initalization of circulary references services (when they are all in stash)
• logic in constructor is anti-patters but I still see it in beginners code quite often, if you do not do any real work in constructor or methods called during service setup it is ok (I wrote that before)
• if this cause error or exception during service setup then again everything is ok (you get error and you know that you need to fix something in service setup)
• problem occurs if this setup error will be silent and will cause problems later, after service is initialized. In my example when somebody try to use $something any time after services are created.
• For such cases I would prefer fail-fast with circular reference exception during service setup.

In my opinion you should explicitly state that some service can be passed to it's dependencies during setup partially initalized. Therefore is you meet conditions described in my example, you will see in code that this service is explicitly allowed passed partially initialized. It it is problem for your code you can change it's structure, setup order or soemthing to solve it.

For example with something like:

$builder->addDefinition($this->prefix('apiClient'))
    ->setClass('Google_Client')
    ->addSetup('setIo', array($this->prefix('@apiIo')));
    ->deferredSetup()

or:

serviceA:
  class: \My\ServiceA
  deferredSetup: true

(I'm not sure about right name – it could be something like lazySetup, allowCircular, …)

Similarly I think that you should get warning that you have circular reference. It it will solve it silently you can miss that fact.

But maybe this would be such exceptional conditions it does not make sense to complicate service creation with it and just let container to solve it for you automatically.

Last edited by mystik (2014-05-29 14:32)

I will try to prepare it.