search

phpro / zf-doctrine-hydration-module

Configurable Doctrine hydrators for ZF2
18 stars 33 forks source link

Remove document from getDoctrineHydrator #25

Closed Bilge closed 8 years ago

Bilge commented 8 years ago

This code doesn't make any sense. The constructor for DoctrineObject takes an ObjectManager and a boolean. Why, then, are you passing a string class name to the second parameter? Presumably there can be no good reason.

Moreover, this method is still bad because you're invoking a concrete instance of DoctrineObject for recursive hydration even though that instance will be different to the original hydrator and may exhibit different behaviour. If you want to do recursive hydration, why not pass the original hydrator instance?

veewee commented 8 years ago

As you might have noticed, this module is existing for quite some time now. It was developed against doctrine module version 0.7.2 which did have a targetClass as constructor argument: https://github.com/doctrine/DoctrineModule/blob/0.7.2/src/DoctrineModule/Stdlib/Hydrator/DoctrineObject.php#L71

This is also the answer to the question about recurrsive hydration.

Can you take a look at the failing tests?

Thanks for noticing this issue!

veewee commented 8 years ago

Thanks! I made some small adjustments and merged.