Change event handling methods

[prolic]

Currenty we use something like this:

protected function apply(AggregateChanged $e)
{
    $handler = $this->determineEventHandlerMethodFor($e);

    if (! method_exists($this, $handler)) {
        throw new \RuntimeException(sprintf(
            'Missing event handler method %s for aggregate root %s',
            $handler,
            get_class($this)
        ));
    }

    $this->{$handler}($e);
}

New approach with be like this:

protected function apply(AggregateChanged $event) {
    switch (get_class($event)) {
    case ManagerWasBlocked::class:
        $this->whenManagerWasBlocked($event); // delegate to when...-method
        break;
    case ManagerWasDeleted::class:
        $this->state = ManagerState::DELETED(); // handle immediately
        break;
    default:
        throw new \UnexpectedValueException('Unknown domain event');
    }
}

Reasons:

• performance
• possibilty to call when-event-name-methods within is still possible
• use of custom event names (f.e. using dots) is easy

refers to: prooph/service-bus#109

[oqq]

This issue depends on an open PR, which benchmarks the apply method. I will provide more cases, especially for a switch construct. #32

I would like to decide if a event is handled by an "on" method in the aggregate. With the current implementation i have to create always an empty 'when' method, if not. With your approach that would be possible. So +1

[prolic]

@oqq Please note this: In the event sourcing lib, there are less problems, as we always expect the event to be an instance of AggregateChanged. But in the service-bus a message (command, event or query) doesn't have to be an object. It can also be an array or string. So here an onEvent-method helps really a lot to handle those situations for listeners.

[oqq]

Ok, so i write benchmarks for event sourcing with the assume it would be an instance of AggregateChanged. I have started to test against strings. Thanks for the hint. :)

[sandrokeil]

Looks good. Migrating existing code to this is also easy. You have to write a little bit more code, but the flexibility/performance is worth it.

@oqq You find some extended bench examples here.

[oqq]

We could provide some examples in docs, so the user could catch from templates for the implementation. Maybe with a reference to benchmarks.

[bweston92]

Prefer this method, looks more functional too.

[codeliner]

👍 for the approach
you could also use the old approach as fallback in the default case so users don't need to migrate all their aggregates at once and prooph-cli would continue to work if you use it.

[prolic]

Fallback is already available in this repo:

protected function apply(AggregateChanged $e): void

it's protected an can simply be overridden in userland code to achive the "new style".