Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow pass ArrayAccess objects to CallableMap #64

Closed
wants to merge 5 commits into from

Conversation

lezhnev74
Copy link

Hi!

As discussed in #60

I have been trying different approaches to solve this problem and it seems to me that "StrategyPattern" is something applicable in this case.

Currently, to make an explicit map I instantiate an object SimpleBus\Message\CallableResolver\CallableMap which expects array and SimpleBus\Message\CallableResolver\CallableResolver as constructor arguments.

I thought it would be great to allow developer to pass ArrayObject or ArrayAccess instead of simple primitive array.
See:

There is a pickle because currently your function expects array:

you would have to typehint for ArrayObject instead of Array. There are inconsistencies in the PHP API, which are extremely frustrating but a part of how PHP works.

Having this option I would pass this kind of command handler FQCN resolver:

class CommandHandlerMapper extends \ArrayObject
{
    public function offsetExists($index)
    {
        return class_exists($this->mapCommandToHandler($index));
    }
    
    public function offsetGet($index)
    {
        return $this->mapCommandToHandler($index);
    }
    
    private function mapCommandToHandler(string $command_FQCN)
    {
        //This is my strategy to map Command to Handler class name
        return $command_FQCN . "Handler";
    }
}

I've prepared a PR for this purpose.
New test is here: tests/Handler/CallableMap/CallableMapTest.php

@matthiasnoback what do you think?

@cmodijk cmodijk self-assigned this Jun 14, 2017
@SimpleBus SimpleBus deleted a comment from coveralls Oct 30, 2017
@cmodijk
Copy link
Member

cmodijk commented Oct 30, 2017

Hi @lezhnev74 thanks for taking the time to create this pull request. Sorry that i toke me a while to get back to you.

What i don't like in this implementation is the fact that we are extending a ArrayAccess to get this to work. If i understand your question you want to be able to resolve a class name instead of relying on the mapping table. But i believe you should already be able to do so with the following class as the resolver.

use SimpleBus\Message\Handler\Resolver\MessageHandlerResolver;

class ClassBasedMessageHandlerResolver implements MessageHandlerResolver
{
    public function resolve($message)
    {
        return get_class($message) . 'Handler';
    }
}

@cmodijk
Copy link
Member

cmodijk commented Dec 22, 2017

I'm closing this for now @lezhnev74 if you need anything let us know

@cmodijk cmodijk closed this Dec 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants