Error while loading objects from mapper

[nawarian]

Like described under #69 the way we instantiate objects that came from a database loading, we're now using Reflection and instantiating without using __construct.

This broke most part of my code logic, and I actually need my constructor being called while loading from repository.

This can be seen under Respect\Relational\Mapper, lines 460 to 462.

Does anyone else have the same problem?

Wouldn't that be better done from a config or something like?

-- EDIT Indeed, wouldn't that be better if we just use instantiation with args? Or make our construct params optional? `... function construct( $arg1 = null )`

http://php.net/manual/pt_BR/reflectionclass.newinstanceargs.php

[felipecwb]

oh shit...

I had think about it that week, but not manifested.

Well you can use previous commit in composer: "respect/relational": "dev-master#74f914f2775955de2170e5054d3cd6896c90390e" or extend Mapper and overwrite the getNewEntityByName()

[felipecwb]

ping @henriquemoody @jackmakiyama

[nawarian]

Agreed. But will it stay like this forever?

Don't think is a good solution...

[nawarian]

We could use some annotations to define constructor params... how about that?

[henriquemoody]

Actually #69 must be reverted.

We can define global arguments to entities' constructors, but maybe it will not fix @jackmakiyama's problems if he has different arguments on entities' constructors. Since the constructors of @jackmakiyama's entities accepts some arguments he must extends Mapper to define the proper arguments on constructor or just instantiate without constructor.

@nawarian, can you elaborate?

[jackmakiyama]

I think entityNamespace property just work with entity classes without contructor, thinking about it, disabling the contructor in the instance by Reflection we can use entites like this.

But I could be wrong. What do you guys think?

[nawarian]

I can make this fix.

But I think it would be nice to discuss first about how should it stay.

Take a look at this sample. It describes what I first told you guys. Will we discuss a better solution or just revert the PR made in #69 ?

[henriquemoody]

I sent #73.

I think constructors are very important we cannot disable them by default.

How about you, guys?

[felipecwb]

I agree with you @henriquemoody.

@nawarian you said about use annotations to define constructors params. Can you explain how will you do that and possible BC breaks?

[nawarian]

Agreed.

About annotations on constructors: probably would be better stay without them. Would be very nice, but complicated for the framework developers and its users. Anyways, thought about something like:

protected $id, $another_object_id;
/**
 * @Relational\ConstructorWithParams
 * @Relational\Params([$another_object_id])
 **/
public function MyClass( AnotherClass $anotherObject )
{
    $anotherObject->doSomething();
}

As I said... much complicated, maybe not useful.

@henriquemoody 's solution appears to be better.

[henriquemoody]

This fix is already available on 0.8.0 version.

@jackmakiyama, you can disable constructor calls doing:

$mapper->disableEntityConstructor = true;

PS.: I have no intention to update documentation with this information cause it seems not the right thing to do in this scenario, but since this feature is made already I don't think rollback is a good idea either.

[jackmakiyama]

Hi guys... o/

Thanks for the fix, it's a very good solution.

I yet think in the future we can remove instantiation for reflection and instantiate the entity directly by fetch mode of PDO.