phpro / zf-doctrine-hydration-module

Configurable Doctrine hydrators for ZF2
18 stars 33 forks source link

Allow instantiable strategies (with no deps) #19

Closed akomm closed 8 years ago

akomm commented 8 years ago

Hydrator strategies are currently fetched only from service manager:

https://github.com/phpro/zf-doctrine-hydration-module/blob/master/src/Service/DoctrineHydratorFactory.php#L288

The factory should try to instantiate a class (if it exists) after failing to fetch it from the service manager. Currently you have to add the strategies to "invokables" of the service manager.

veewee commented 8 years ago

Sorry for the late response, I was on a 2 weeks vacation. I would rather keep the configuration explicit: if you want to fetch a strategy from the service manager, you will have to register it to the service manager. This way it is easier to debug what is going on.

akomm commented 8 years ago

It just adds up boilerplate to things, which work normaly out of the box. In case of simple instantiations both would lead to the same result on failure, but the version with service manager would have one more exception thrown and catched.

veewee commented 8 years ago

Hello @akomm, This is not something that works out of the box in the ServiceManager. For the AbstractPluginManagers this is a configurable option named $autoAddInvokableClass which is true by default. The strategies are located in the main ServiceManager, which doesn't seem to have any auto invoking logic:

mkdir /tmp
cd /tmp
composer require zendframework/zend-servicemanager

Test:

require 'vendor/autoload.php';
$sm = new Zend\ServiceManager\ServiceManager();
$sm->get('stdClass')

Result:

Zend\ServiceManager\Exception\ServiceNotFoundException with message 'Zend\ServiceManager\ServiceManager::get was unable to fetch or create an instance for stdClass'
>>> exit

The extra configuration boilerplate does not bother me, I like to keep it explicit. The service manager should not do too much magic like auto initialization for me.

akomm commented 8 years ago

I see your point. I'm actually rather an 'explicit'-guy too. But i'v noticed that it sometimes draw you back for almost no reason, so I became more pragmatic. But the point with autoAddInvokables is a good one so no reason to make this change :)