[DI] Add section about getter injection #7300
Conversation
|
|
||
| The disadvantages of getter injection are: | ||
|
|
||
| * You must call the getter everytime you need the dependency internally. |
There was a problem hiding this comment.
There isn't any property holding the dependency to use anyway. Is it worth mentioning this as a drawback? 😅
There was a problem hiding this comment.
That's actually the same drawback as outlined here, reformulated :)
There was a problem hiding this comment.
I meant this should probably not appear in the drawbacks section at all. It isn't one. Just a consequence of the above. Which has no impact.
There was a problem hiding this comment.
oh! removed then!
| app.newsletter_manager: | ||
| class: AppBundle\Mail\NewsletterManager | ||
| getters: | ||
| - getMailer: '@mailer' |
There was a problem hiding this comment.
Shouldn't it rather be:
app.newsletter_manager:
class: AppBundle\Mail\NewsletterManager
getters:
getMailer: '@mailer'
getLogger: '@logger'?
f9144d5 to
f8d3457
Compare
| // ... | ||
| abstract class NewsletterManager | ||
| { | ||
| abstract protected function getMailer(): MailerInterface; |
There was a problem hiding this comment.
If these return types are mandatory, we must add somewhere that this only works with PHP 7. Otheriwse, we should remove them.
There was a problem hiding this comment.
I forgot to mention that PHP 7 is required when using getter injection. Added now.
About the type hints, they are not required, but highly recommended. Given the PHP 7 requirement, I think we should keep them here.
f8d3457 to
801ea33
Compare
801ea33 to
2d32d66
Compare
| ---------------- | ||
|
|
||
| .. versionadded:: 3.3 | ||
| Getter Injection was introduced in Symfony 3.3. |
There was a problem hiding this comment.
We should mention that this is an experimental feature
| Getter Injection | ||
| ---------------- | ||
|
|
||
| .. versionadded:: 3.3 |
There was a problem hiding this comment.
The leading spaces should be removed.
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Add getter injection | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20657 | License | MIT | Doc PR | symfony/symfony-docs#7300 Getter overriding by the container will allow a new kind of dependency injection which enables easier laziness and more immutable classes, by not requiring any corresponding setter. See linked issue for more. This is WIP: - [x] wire the concept - [x] dump anonymous classes with PhpDumper - [x] generate at runtime in ContainerBuilder::createService - [x] tests - [x] make it work on PHP 5 Commits ------- cb49858 [DI] Add getter injection
This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Add getter injection | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20657 | License | MIT | Doc PR | symfony/symfony-docs#7300 Getter overriding by the container will allow a new kind of dependency injection which enables easier laziness and more immutable classes, by not requiring any corresponding setter. See linked issue for more. This is WIP: - [x] wire the concept - [x] dump anonymous classes with PhpDumper - [x] generate at runtime in ContainerBuilder::createService - [x] tests - [x] make it work on PHP 5 Commits ------- cb498580d1 [DI] Add getter injection
|
|
||
| // ... | ||
| $container->register('app.newsletter_manager', NewsletterManager::class) | ||
| ->addOverriddenGetter('getMailer', new Reference('mailer')) |
There was a problem hiding this comment.
Should be setOverriddenGetter (same below).
|
Reverted, one less thing to document for 3.3 :) |
Ref symfony/symfony#20973