Is it a good practice to load Models as a service like I do when I use Symfony framework?

Question

Usually on Symnfony framework I use the provided depedency Injection container and on Controller I load the models as a service. To be more In detail on a current project I am having the following services.yml:

# Learn more about services, parameters and containers at
# http://symfony.com/doc/current/book/service_container.html
parameters:
#    parameter_name: value

services:
#    service_name:
#        class: AppBundle\Directory\ClassName
#        arguments: ["@another_service_name", "plain_value", "%parameter_name%"]

##################### Crud Managers ##########################
  pet_manager:
    class: AppBundle\Managers\CRUD\PetManager
    arguments: ["@doctrine.orm.entity_manager"]

  person_manager:
    class: AppBundle\Managers\CRUD\PersonManager
    arguments: ["@doctrine.orm.entity_manager","@pet_manager"]

############################ Models ###########################

  person_model:
    class: AppBundle\Models\PersonModel
    arguments: ["@person_manager"]

That On my model I have the basic Business Logic and I use managers in order to fetch data from db or for performing third party actions such as interfacing a third party api, handling filesystem etc etc. All managers are getting Injected into the Models via Depedency Injection.

In Symfony on my controller I use:

$personModel = $this->get('person_model');

In order to load the model.

But is this considered a good practice-approach-architecture in order to apply into another frameworks or vanilla php applications? What caveats may have?

Answer 1

Couple of things.

Try to avoid generic suffixes such as Manager or Model. They really don't indicate what the class is for. And indeed, @Richard87 thinks PersonModel is an individual person entity when it clearly is not.

Having your manager objects depend on other managers is not going to scale. You can easily end up with three,four,five managers each depending on each other. Instead try to think in terms of domains models in which each "manager" is more or less stand alone. So if the domain involves people with pets then setup a manager to deal with both of those entities.

Avoid injecting the entity manager when possible. Instead inject repositories as that at least partially limits the scope of each class. In fact, instead of a manager you might be able to add your logic directly to the repository class.

Finally, if you want to achieve some level of framework independence (and perhaps better testing) then consider defining your controller actions as services (the docs tell you how). Doing so eliminates the need for the service locator pattern and the need for framework specific code such as $this->get(). Your actions just get injected with whatever they need to do their work.

Answer 2

No, its not a good practice.

First of all, your domain objects should not know anything about persistence...

Second of all, the Object's state will be in a invalid state if you load the object from a database (the constructor will not be called and the repository will not be injected).