zenstruck / console-extra

A modular set of features to reduce configuration boilerplate for your Symfony commands.
MIT License
78 stars 3 forks source link

createUser example and best practices #64

Closed tacman closed 6 months ago

tacman commented 8 months ago

Since I study your bundles to learn new techniques and best practices, I'd like to ask about the example in the README. Indeed, I have several commands like this, to create entities from the CLI with some options.

The business logic in the example comes down to

$repo->createUser($email, $password, $roles);

Because there's no flush in the command, one presumes that the repo is creating, persisting and flushing the entity. I think at one point Symfony even starting putting that logic in the MakeEntity in the maker bundle, with an optional $flush parameter.

Obviously, you want the example to be simple, but since some people are opposed to flushing in the repository, presumably you'd have it here. And probably the repository class as well.

I bring all this up as a backdrop for saying that in my experience using this, I've found that injecting services into the constructor being a better practice that in invoke for anything beyond simple cases. BUT maybe I should change my approach, and put more of the business logic into a service that's passed in __invoke, as you've done in the example.

A typical command for me involves refreshing data from an API, so I have methods that load the existing data from the repository (so I don't have to do a ->findOneBy() for each item), then creating and persisting each new entity, then updating the new/existing entity with the fetched data. Pretty quickly I create methods for these, which means I need the repositories and entityManager.

As far as best practices, should that logic be moved to a service? Or, if it's in a command (which is of course a service), then the services would be in the constructor (or passed from invoke to each method).

Again, I realize that either method works, but since V2 is imminent I was thinking more examples that showed off best practices would be useful to me.

kbond commented 8 months ago

Because there's no flush in the command, one presumes that the repo is creating, persisting and flushing the entity.

Indeed, this is not the recommended practice. I've updated the docs to inject a UserManager service.

I've found that injecting services into the constructor being a better practice that in invoke for anything beyond simple cases.

I guess this is the same for services injected into controllers. Assuming the service isn't needed in multiple methods in the same class, I prefer injecting into the method directly.

For instance, in the create user command, if you had a suggestions method that required fetching roles from the UserManager, I'd inject in the constructor only.

tacman commented 6 months ago

thx for updating the example.