Proposal of adapter constructor changes

[prolic]

Currently we pass an array of options to any adapter constructor.
What would you think of passing a configuration object? You can still have an array of options, but you would pass them to the config object, instead of the adapter directly.

Something like this:

$config = new Config($someArray);
$adapter = new SomeAdapter();
$config->configureAdapter($adapter);

This is the same approach zf2 takes for the service manager, see: https://github.com/zendframework/zend-servicemanager/blob/master/src/Config.php

Downsides: We have to add public getters and setters to any adapter. For security reasons, we could disallow setting them twice (throwing an exception).

If accepted I could take this job over the weekend.

Thoughts?

[codeliner]

Sounds good 👍 We have such a config object for the event store (inspired by zf service manager config ;)). But I would like to avoid setter injection. What about passing the adapter options to a factory instead? I'm currently working on turning proophessor ServiceFactories into invokable container factories: see #57 Maybe something like that can work for the adapters too? We can also discuss this in the chat if you like: https://gitter.im/prooph

[prolic]

Not today, but tomorrow perhabs
Am 14.08.2015 23:12 schrieb "Alexander Miertsch" notifications@github.com:

Sounds good [image: 👍] We have such a config object for the event
store (inspired by zf service manager config ;)). But I would like to avoid
setter injection. What about passing the adapter options to a factory
instead? I'm currently working on turning proophessor ServiceFactories into
invokable container factories: see #57
#57 Maybe something like
that can work for the adapters too? We can this also discuss in the chat if
you like: https://gitter.im/prooph

—
Reply to this email directly or view it on GitHub
#60 (comment).

[codeliner]

I'm off tomorrow :). Let's do it next week.

[prolic]

Okay, no problem. Have a nice weekend.

[codeliner]

you too

[codeliner]

@prolic After reading (and writing) a lot about framework interoperability the last days I'd like to come back to your proposal.

The next days I want to focus on writing more "container-aware factories". I nearly finished the task for the service-bus component: prooph/service-bus#57
and will do the same for the event-store and maybe also for the adapters.

However, these factories should not be the only helpers to set up prooph components. They'll only work fine in environments like ZF2 and hopefully also in zend-expressive (which I definitely want to target).

So your configuration object proposal is really interesting. We should get rid of passing an array to a adapter constructor. But replacing it with setter injection is not an ideal solution. An adapter requires a connection so this must be passed via constructor.
All adapters will require a MessageFactory and a MessageConverter when prooph/event-store 5.0 is out.
The SQL-ish adapters also require a PayloadSerializer.

What do you think about providing an AdapterBuilder instead of a configuration object:

$adapterBuilder = new MongoDbAdapterBuilder(/*optionally, you can pass array config here*/);

$adapter = $adapterBuilder->use($connection)->use($messageFactory)->build();

If for example a MessageConverter is not given the builder falls back to the default one.
What do you think?

[prolic]

A builder is not required. The factory will fetch the config from the service locator, create all needed dependencies put them in the constructor. Every adapter has its own constructor and the factory knows how to create an instance. That's all.

I'll try to provide the needed code changes for the event-store, asap.

[codeliner]

Great 👍 . Then let's go with the factory approach only. Reduces implementation and documentation effort.
Some people will say "oh my god they use service location. That is poor practice."
But I'm a big fan of the ZF2 servicemanager factories and porting them to container-aware factories is really straightforward.

[prolic]

Service location in a factory is absolutely okay. Problem only arises when
used in the Implementation.
Am 18.08.2015 20:34 schrieb "Alexander Miertsch" notifications@github.com:

Great [image: 👍] . Then let's go with the factory approach only.
Reduces implementation and documentation effort.
Some people will say "oh my god they use service location. That is poor
practice."
But I'm a big fan of the ZF2 servicemanager factories and porting them to
container-aware factories is really straight forward.

—
Reply to this email directly or view it on GitHub
#60 (comment).

[codeliner]

Absolutly agree. Just read through the FIG mailing list today to find out why PSR-11 is not accepted yet. Seems like some people consider all kinds of service location as bad practice even when only used in a factory. Don't ask me why. Seems like in PHP everything must be done in at least 10 different ways instead of just relying on some standards and conventions that simply work.