thephpleague / tactician-bundle

Bundle to integrate Tactician with Symfony projects
MIT License
245 stars 43 forks source link

Zero-configuration for command handlers #98

Closed petr-buchin closed 6 years ago

petr-buchin commented 6 years ago

For now user must manually add tag "tactician.handler" with attribute "typehints" set to true in order to wire a handler with a command by typehint. But with Symfony's autoconfiguration and autowire features we can have zero-configuration for command handlers. All we need is:

  1. Add some interface for command handlers (it does not even need to have any methods), for example:
namespace League\Tactician\Bundle;

interface CommandHandlerInterface 
{
}
  1. Register this interface for autoconfiguration in DependencyInjection extension class:
$container
    ->registerForAutoconfiguration(CommandHandlerInterface::class)
    ->addTag('tactician.handler', ['typehints' => true]);

After that we can create our command handlers and wire them to commands without any configuration.

I will be happy to create a PR, but I should know, where to create CommandHandlerInterface - in a bundle or in main tactician repo itself.

rosstuck commented 6 years ago

sHi @petr-buchin, thanks for opening up a well explained proposal (and for using Tactician!) 😄

The idea of autowiring based on marker interfaces has come up before. However, I'd rather not encourage using marker interfaces. In an ideal situation, Tactician would live behind a bridge interface defined in the user's own namespace and Tactician would not be visible in the code you're implementing. This is true for the base League\Tactician\CommandBus itself but also preferably for the handlers as well: there's no required interface for them.

We could have it as an optional feature but one thing I've noticed is that the existence of any feature condones the practice. 😅

As you say though, this is fairly easy to implement using the PHP code above or using a filename wildcard in the resource key of the DI container. I think that's a nice way to support this at an 80% level and the config needed is documented now. It isn't a perfect fit for every directory structure but the goal of Tactician isn't to directly support every configuration but make it easy to have each project implement its own configuration.

I hope that explains the reasoning behind this a bit. I hate being one of those maintainers that immediately shoots down an well-written idea once it comes in but I hope you understand where I'm coming from.

I'm going to close the issue for now but if you'd like to discuss further, feel free to reopen or post further. Thanks again! :)

petr-buchin commented 6 years ago

Hi, @rosstuck , thanks for a detailed answer!

I hate being one of those maintainers that immediately shoots down an well-written idea once it comes in but I hope you understand where I'm coming from.

Don't worry, I've read your answer, and after this sentence:

Tactician would live behind a bridge interface defined in the user's own namespace and Tactician would not be visible in the code you're implementing

i realized that you are right, and the better approach is to implement such a thing in the app itself :)

Again, thanks for the explanation!

rosstuck commented 6 years ago

More than welcome, good luck! :)