Skip to content
This repository has been archived by the owner on Jul 16, 2021. It is now read-only.

[Proposal] Add method to allow resolving abstract with specified context for it in Container #1477

Open
d3jn opened this issue Jan 9, 2019 · 3 comments

Comments

@d3jn
Copy link

d3jn commented Jan 9, 2019

Whenever we resolve the abstract it's dependencies can be resolved with contextual binding, but never the abstract itself. Sometimes there is a need to delay the actual construction of object so we can't reference it in __construct, but we want for it to be resolved contextually for this class as a dependency when we actually need it (and if we need it).

One of the workarounds for it is this idea (tl;dr - lazyloading/proxying the dependency) - #1436

I've always felt like enforcing context onto Container's make() could be another way to go around this. Then we can construct object when we need it and we provide context to container so that it knows what class is in need. For example:

app()->makeWithContext('App\Contracts\CodeGenerator', ['length' => 5], get_class($this));

Or if context is not specified we could retrieve it using debug_backtrace(), so that we can just call:

app()->makeWithContext('App\Contracts\CodeGenerator', ['length' => 5]);

from within our class that is dependent on (in this case) App\Contracts\CodeGenerator, but doesn't need it all the time. So we are constructing/using it only when needed and can also lazy load it manually etc. While featuring it as dependency injection in constructor could lead to big overhead (esp. with specific implementations that rely on external services, interacting with file system and so on upon construction) and this is what this proposal battles.

What's your thoughts on something like that? Doesn't look like it will take much to implement, I am ready to do a PR myself, but would want to hear some critique/more ideas on the matter first.

@Patryk27
Copy link

Patryk27 commented Jan 9, 2019

As far as I understood you, creating such proxies or delegates usually means that you've got yourself a pretty big pile of architectural mess - circular dependencies between classes should be avoided, simply because it's impossible to correctly instantiate them.

For example - there's no way to instantiate neither Foo nor Bar in this case:

class Foo {
  public function __constructor(Bar $bar) {
    /* ... */
  }
}

class Bar {
  public function __constructor(Foo $foo) {
    /* ... */
  }
}

new Foo(new Bar(new Foo(?)));

You could try to perform a setter DI:

class Foo {
  public function __constructor(Bar $bar) {
    /* ... */
  }
}

class Bar {
  public function setFoo(Foo $foo) {
    /* ... */
  }
}

$bar = new Bar();
$foo = new Foo($bar);
$bar->setFoo($foo);

... but that's not actually a solution - it defies the entire point of constructor, which is: to construct a correct, usable object. This new Bar() creates an unusable instance of the Bar class and so its constructor is somewhat impaired.

With that being said, Symfony does give a way to perform a setter DI (https://symfony.com/doc/current/service_container/injection_types.html), but I'm really not sure we should go that way - IMO circular dependency is a problem that should be solved inside the application, not by framework.

@vv12131415
Copy link

@Patryk27 How do you think it should be solved in the application? Just avoid it?

@Patryk27
Copy link

Patryk27 commented Jan 10, 2019

@vladyslavstartsev: I'm aware that just don't do circular dependency may seem like a vague term (usually it's hard to predict that some classes are going to cause mayhem in the future), but I'd pretty much say so: avoid circular dependencies when you notice them. If you have a case similar to that Foo & Bar example I've given before, Just Refactor It (TM).

I've myself had a few cases of circular dependency between classes and as far as I remember, a quick refactoring always helped - even though at first I actually did think Why cannot I just instantiate them and have this off my mind? I've got another task to do!, I eventually realized that this precise code could break again in future and I'd have really hard time understanding what's going on. Especially if proxies or delegates were involved, which could cause additional bugs on their own.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants