Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Still continue appear message warning: Deprecated #89

Closed
wtabata opened this issue Mar 2, 2016 · 11 comments
Closed

Still continue appear message warning: Deprecated #89

wtabata opened this issue Mar 2, 2016 · 11 comments
Labels

Comments

@wtabata
Copy link

wtabata commented Mar 2, 2016

In " \zend-mvc\src\Controller\AbstractController.php on line 258 "

Let me see if I got it right. I can not use getServiceLocator() in the controller? I have to send all pendencies by factories? Even extending AbstractActionController?

@weierophinney
Copy link
Member

@wtabata Yes. We're emitting it at that location for a number of reasons:

  • We had to remove the ServiceLocatorAwareInterface implementation from AbstractController due to the fact that it does not exist in zend-servicemanager v3; we want developers to be able to upgrade to that version if desired, but we could not do so while still implementing that interface.
  • For backwards compatibility purposes, we need to still inject the service locator, even if users never consume it. Doing so, however, should not emit a deprecation notice, because we cannot know if it is even being used.
  • When developers call getServiceLocator() from within their controllers, then we know they are using it, and that's when we need to raise the deprecation notice.

Because ServiceLocatorAwareInterface is removed from zend-servicemanager v3, and because we plan to remove the initializers for it in zend-mvc v3, we are using the deprecation notices to signal to users when and where they need to make changes to their code to make it forwards compatible.

We have been recommending for literally years that developers not pull dependencies from the composed service locator, and instead directly inject their dependencies. We recommend this for several reasons:

  • Explicitness. If you require dependencies via your constructor and/or setters, you can tell by the method signatures precisely what your class requires in order to operate. This allows you to know what components, libraries, modules, etc. you need to install.
  • Testing. It's far easier to test when you can directly inject your collaborators, vs. injecting into a service container, and injecting the service container. Additionally, without explicit dependencies, writing tests becomes cumbersome as you need to understand the code internals to know what dependencies will be fetched from the service container for any given code path or behavior.

While we recognize that during early development of a controller, it's often easier to pull stuff out of the service locator, we feel that for the reasons stated above, it's an anti-pattern, and will lead to costly maintenance eventually. For these reasons, we'll be removing this capability in zend-mvc v3.

If you rely on that capability, you have several choices:

  • Pin to an earlier version of zend-mvc (versions earlier than 2.7). Be aware, however, that these may not continue to receive security updates.
  • Use zend-mvc 2.7, and either disable reporting of deprecation notices via the error_reporting INI configuration or the display_errors configuration.
  • When v3 drops, you can always add your own initializer for this capability. However, you will also need to re-add the methods for injection and retrieval of the service locator in your controllers.

Alternately: start updating your code to remove this usage, as your code and maintenance will benefit from the changes.

@wtabata
Copy link
Author

wtabata commented Mar 2, 2016

@weierophinney terrific!!

@tasmaniski
Copy link

Thanks for the extensive explanation!

But still I have an issue regarding to "Alternately: start updating your code to remove this usage...".

I already have all dependencies injected via factory in constructor and I never use getServiceLocator() method (but I am using ControllerPlugins).
Should we remove "MyController extended AbstractActionController" ?
I didn't found in migration guide clearly what should be the replacement and best practice for this case ?

@weierophinney
Copy link
Member

Which plugins are you using, specifically?
On Mar 3, 2016 11:29 AM, "Alex" notifications@github.com wrote:

Thanks for the extensive explanation!

But still I have an issue regarding to "Alternately: start updating your
code to remove this usage...
".

I already have all dependencies injected via factory in constructor and I
never use getServiceLocator() method (but I am using ControllerPlugins).
Should we remove "MyController extended AbstractActionController" ?
I didn't found in migration guide clearly what should be the replacement
and best practice for this case ?


Reply to this email directly or view it on GitHub
#89 (comment)
.

@tasmaniski
Copy link

Redirect, FlashMessenger, Identity, Params and Layout.
I don't have message warning: Deprecated anymore, I am just curious about migration to new zend-mvc.
Should we do something else except remove usage of getServiceLocator() in Controller.

@evghen1
Copy link

evghen1 commented Mar 4, 2016

Small offtopic, but related:

you can tell by the method signatures precisely what your class requires in order to operate.

What possibilities to sign setter to be clear is required for this class (by constructor is clear)?

@ronan-gloo
Copy link

Hi,

We have been recommending for literally years that developers not pull dependencies from the composed service locator, and instead directly inject their dependencies.

Into a business (or service, or model) layer, i'm more than agree with this assumption, dependencies must be required and explicit as much.

But for front controllers, as some MVC layers promote them, constructor dependency injection can be a huge waste of resources.

Given a typical CRUD controller, where the post / update action requires a form as dependency. Why should we inject a sometimes very sophisticated form into the controller, when we just need to perform a list or get action ?

While some of our controllers does not respect some fundamentals from the single responsibility design principle, i'm convinced that strict DI is not a design pattern to apply on, and the use of the service locator seem's a fair trade.

The default action based zend-expressive approach helps a lot to solve this, by writing consistent request-front objects :)

@weierophinney
Copy link
Member

@ronan-gloo We provided an answer to that scenario (conditional dependencies) in ZF 2.3, via lazy services. The recommendation is to use lazy services when you have a dependency that is only used in some code paths; these are injected as normal dependencies, i.e., via constructor or setter injection. The benefit they provide is that the "heavy" service is not retrieved from the service container until first access of any method on the service instance.

So, yes, the recommendation of injecting all services vs pulling them from a composed service locator still stands. The addition of lazy services to zend-servicemanager was what allowed us to unequivocally recommend the practice.

@dionisiolouro
Copy link

Hi,
how should I order the Doctrine\ORM\EntityManager now? The documentation recommends that you do the getServiceLocator ( )

@NeftaliYagua
Copy link

I think we should correct documentation with the correct method, so we avoided continue to develop wrong code.

@Ocramius
Copy link
Member

Hey Doug,

The fact that you pre-inject things that you won't use indicates some SRP
issues. Regardless of that, you can still just make the injected services
lazy, which will prevent any init-time I/O

Marco Pivetta

http://twitter.com/Ocramius

http://ocramius.github.com/

On 30 March 2016 at 03:04, Doug Bierer notifications@github.com wrote:

Hate to sound like a dinosaur ... but by forcing developers to pre-inject
everything, we're back to the good old create a base class and extend it
paradigm. The idea of inversion-of-control is that we're not bogging down
classes with unneeded functionality, right? So if I have a controller where
1 method needs X and 4 methods do not, by pre-injecting X I'm wasting
resources 80% of the time.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#89 (comment)

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

No branches or pull requests

8 participants