Removing type hint from CommandHandler

[jenkoian]

Hear me out..

I'm working on an example hexagonal app using commands and handlers. I wanted to introduce a command bus into the equation and liked the look of this.

One of the big concepts I want to put across in the application is the dependency rule and ensuring that we're only depending on inner dependencies.

So I'm creating adapters for any outer dependency. Therefore to use this library, I have something that looks like the following:

Example Command:

<?php

namespace Jenko\House\Command;

use Jenko\House\Adapter\SimpleBusCommandAdapter;

final class EnterRoomCommand extends SimpleBusCommandAdapter
{
    const NAME = 'enter-room';

    public $house;
    public $room;
}

Notice it's extending the adapter, which looks like this:

<?php

namespace Jenko\House\Adapter;

use Jenko\House\Command\CommandInterface;
use SimpleBus\Command\Command;

abstract class SimpleBusCommandAdapter implements Command, CommandInterface
{
    /**
     * {@inheritdoc}
     */
    public function name()
    {
        return static::NAME;
    }
}

Pretty straight forward so far. So then I wanted to do a similar thing with handlers:

Example Handler:

<?php

namespace Jenko\House\Handler;

use Jenko\House\Adapter\SimpleBusHandlerAdapter;
use Jenko\House\Event\EventDispatcherInterface;
use Jenko\House\Factory\HomeAloneHouseFactory;
use Jenko\House\House;

final class EnterRoomHandler extends SimpleBusHandlerAdapter implements HandlerInterface
{
    /**
     * @var House $house
     */
    private $house;

    /**
     * @var EventDispatcherInterface
     */
    private $dispatcher;

    /**
     * @param EventDispatcherInterface $dispatcher
     */
    public function __construct(EventDispatcherInterface $dispatcher)
    {
        $this->house = HomeAloneHouseFactory::getHouse();
        $this->dispatcher = $dispatcher;
    }

    /**
     * @param $command
     * @return mixed|void
     */
    public function handle($command)
    {
        $house = $this->house->enterRoom($command->room);
        $this->dispatcher->dispatch($house->releaseEvents());
    }
}

The Adapter:

<?php

namespace Jenko\House\Adapter;

use Jenko\House\Handler\HandlerInterface;
use SimpleBus\Command\Command;
use SimpleBus\Command\Handler\CommandHandler;

abstract class SimpleBusHandlerAdapter implements CommandHandler, HandlerInterface
{
    /**
     * {@inheritdoc}
     */
    abstract public function handle(Command $command);
}

Now obviously this doesn't work because of the type hint for Command. As SimpleBus\Command\Handler\CommandHandler is expecting a SimpleBus\Command\Command instance, I must type hint for that in my handlers. Thus, coupling my handlers to the SimpleBus\Command\Handler\CommandHandler. Type hinting to my adapter doesn't work unfortunately, as the method signature must match exactly of the interface.

I'm unsure if the commands and handlers should be part of my domain or if they are deemed part of the infrastructure (where I care less about decoupling so much).

So, sorry for the long PR, but I guess it's kind of an RFC with the following questions:

• Can we lose the type hint?
• Should I just not worry about decoupling my commands/handlers so much?
• Other?

I tried to look at what similar libraries do, but didn't come to any conclusion really. Broadway doesn't require a type hint (https://github.com/qandidate-labs/broadway/blob/master/src/Broadway/CommandHandling/CommandHandlerInterface.php) nor does (https://github.com/tabbi89/CommanderBundle/blob/master/Command/CommandHandlerInterface.php) but then other similar libs do so ¯_(ツ)_/¯

[matthiasnoback]

Thanks for the elaborate description of your problem. Several things come to mind:

• The SimpleBus classes and interfaces are very generic and very slim too, so you're not programming against concrete things, but abstract things, and this means that you are following the Dependency Inversion Principle very well (something I think you're trying to accomplish by introducing your own interfaces for these things). So in short: I don't think you should bother having your own interfaces and abstract classes for everything. Just make sure you don't depend on the concrete stuff of other libraries.
• Losing the type-hint does solve your particular issue, but it introduces the problem where people might just throw anything at the command handler and I'd have to check if the input is of the correct type (which is what type-hints are for ;)). So I'm not merging this one, sorry...
• Your strategy for introducing adapters is to use inheritance, while in fact you'd be better off using composition. In other words, you could inject the command handler which satisfies the interface from SimpleBus into a command handler which satisfies your interface. This also solves the issue you described.

I hope this helps!
And thanks for using SimpleBus. If you're happy, you're free to close this issue.

[jenkoian]

The SimpleBus classes and interfaces are very generic and very slim too, so you're not programming against concrete things, but abstract things, and this means that you are following the Dependency Inversion Principle very well (something I think you're trying to accomplish by introducing your own interfaces for these things). So in short: I don't think you should bother having your own interfaces and abstract classes for everything. Just make sure you don't depend on the concrete stuff of other libraries.

I agree, although I would like my domain logic to be completely decoupled as opposed to the loose coupling you talk about. I think overall this is my solution though, to move my commands and handlers outside of my domain in to some kind of command handling layer, or ports layer as I've heard it described elsewhere.

Losing the type-hint does solve your particular issue, but it introduces the problem where people might just throw anything at the command handler and I'd have to check if the input is of the correct type (which is what type-hints are for ;)). So I'm not merging this one, sorry...

Yeah I didn't really expect it to get merged, just wanted to open up the conversation :)

Your strategy for introducing adapters is to use inheritance, while in fact you'd be better off using composition. In other words, you could inject the command handler which satisfies the interface from SimpleBus into a command handler which satisfies your interface. This also solves the issue you described.

Yeah I was uncomfortable with it too. In just seemed odd injecting a handler into a handler.

Cheers for the response, as mentioned I think my solution is to have a dedicated command handling layer which is only loosely coupled to this command bus.

[matthiasnoback]

Thanks. This would have been a nice case for "duck-typing", where having a handle method is quite enough to be considered a command handler and having a name() method is sufficient to be considered a command :). Okay, if you stumble upon any other issue, just let me know!

[jenkoian]

Just for reference - my problem here is more or less the same as this Stack Overflow question: http://stackoverflow.com/questions/26699118/hexagonal-architecture-clean-code-problems-implementing-adaptor-pattern

[matthiasnoback]

@jenkoian Thanks for pointing this out. About adapters: they are not meant to implement both interfaces, they are merely meant to bridge the gap between one interface and another. So, in the case of the console output, MySymfonyDialogAdaptor should wrap the MyOutputInterface instance in a MyOutputToSymfonyOutputAdapter, which implements the Symfony OutputInterface. Of course, you'll need to implement lots of methods, but that's because this interface tries to provide too many methods in the first place (it doesn't do Interface segregation principle ;)).

I still think that you wouldn't need to go as far as you are doing now. The idea of hexagonal architecture (which this library is meant to support), is that you separate adapter code from core code, or technical details of the input and output of your application from your domain code. It's not bad if you use third-party code in the core of your application - you can not write everything yourself. It is however important to make your core point inwards with regard to external system, like a database, or a web browser. It should not be aware of any of those.

[jenkoian]

@matthiasnoback thanks for the reply :)

About adapters: they are not meant to implement both interfaces, they are merely meant to bridge the > gap between one interface and another. So, in the case of the console output,
MySymfonyDialogAdaptor should wrap the MyOutputInterface instance in a
MyOutputToSymfonyOutputAdapter, which implements the Symfony OutputInterface

Unless I'm mis-understanding, I'm not sure I agree here. Doesn't it make more sense to depend on your own interface and adapt to the third party interface (in this case OutputInterface)? (I do agree they shouldn't implement both interfaces, I only did that in my code example (above) to try and get it to work with the existing type hints, the code on the SO post though doesn't implement multiple interfaces though).

This seems right to me. You want to rely on your own interface and use adapters to 'adapt' outside interfaces to fit. I'm not sure I see the need to create an adapter in the other direction, i.e. MyOutputToSymfonyOutputAdapter unless you are specifically working with a class which is expecting an instance of OutputInterface.

EDIT: Having spoken to some colleagues I think I was mis-understanding you here and now see what you mean and can see how that can solve the issue. When I get time today I'm going to put a gist together to clarify it in my own head, but thanks for the hint.

I still think that you wouldn't need to go as far as you are doing now. The idea of hexagonal architecture > (which this library is meant to support), is that you separate adapter code from core code, or technical > details of the input and output of your application from your domain code. It's not bad if you use third-
party code in the core of your application - you can not write everything yourself. It is however
important to make your core point inwards with regard to external system, like a database, or a web
browser. It should not be aware of any of those.

I think this depends on how seriously you are taking the dependency rule and also if you care about loose coupling vs de-coupling

[matthiasnoback]

unless you are specifically working with a class which is expecting an instance of OutputInterface

This is the case, right? In the example the dialog helper explicitly needs such an instance. So then you just have to wrap it again (only problem is the amount of methods that you won't be able to support).

The Dependency rule is interesting, but I'm not sure if the dependency on interfaces of SimpleBus is really in the wrong direction. As I said, you still apply to the Dependency inversion principle by depending on an interface which models a very abstract concept as well. Also, code from a library isn't necessarily part of an outer ring. If you recreate the code in your own project, then it will be in the same ring and you would still not be allowed to depend on it. This would lead to the conclusion that it's no problem to use Command, Event and the handler/subscriber interfaces in the core of your application.

[jenkoian]

Yeah, as mentioned int he edit I think I was mis-understanding initially. This makes sense. In fact I've been talking to Chris (author of the SO post) and we've come up with something like https://gist.github.com/chriscollinsboxuk/c5aa552cdfa8bb55c870 which I think is the point you were trying to make.

Still digesting your second point, but thanks for your input with all this, appreciate you taking the time on essentially a non issue :)

[matthiasnoback]

Exactly, that was what I meant :)

Thanks to you too. I think it's important to explain/defend certain design choices. If I can't do that, then I should try harder/rethink things. And that actually happens a lot, so I always try to follow the reasoning of people commenting to find out if there is a new concept/solution hidden behind their questions.