Handling factory methods using instantiateWith

[1ed]

Hi, I have big entity with lots of attributes. This Entity has different instantiation logic based on the usage, so I use factory methods which have some validation logic. I would like to use these methods in foundry factories too, so I use

// ...
->instantiateWith(function(array $attributes, string $class): object {
    return MyEntity::factoryMethod($attributes['foo'], $attributes['bar']);
})

I'm wondering are there any other option? Because, in this case the other entity properties never initialized based on the attributes. I don't want to call all the setters on the entity manually. Using the Instantiator class is not possible because it will not call the factory method on the entity, but it always instantiates the subject by itself

foundry/src/Instantiator.php

Line 43 in 321abbe

$object = $this->instantiate($class, $attributes);

Maybe we should split this class into 2, one for instantiate and one for initialize the subject, or simply just allow to call factory methods instead of the constructor like:

new Instantiator([MyEntity::class, `factoryMethod`])
// of if we want to keep it reusable 
public function __invoke(array $attributes, string $class, string $method = '__construct'): object
// or setting the instance
Instantiator::withObject($entityInstance)

[nikophil]

Hi @1ed

I think you raise a fair point here.

IMO the cleanest way to handle this would be to split the Instantiator and to introduce something like an Hydrator. The current Instantiator is actually doing both of these tasks

[kbond]

Agreed this is a fair point and splitting the class.

Some thoughts:

1. BC?
2. Probably a Factory::hydrateWith() could be added?
3. Currently, when the instantiator creates the object with the constructor, the attributes are used up (so they aren't used on setters). How could we handle this?

[nikophil]

1- About BC all following options will apply to hydrator and not on instantiator anymore: allowExtraAttributes, extraAttributes, alwaysForceProperties, forceProperties, so we need the instantiator to pass these options to the hydrator, in order that this kind of code to continue to work:

    public function initialize(): self
    {
        return $this->instantiateWith(
            (new Instantiator())->alwaysForceProperties()
        );
    }

and we need to deprecated all setters for these options in the instantiator.

Also I may be wrong, but I think there is no valid use case to use Instantioator::forceGet() and Instantioator::forceSet() in userland and they should have been marked internal? so we should deprecate them as well.

2- agreed, and the hydrator should have all the setters for the options mentioned above.

3- IMO this should be handled the same way than now, given Instantiator::instantiate() is "instantiation phase" and the rest of Instantiator::__invoke() is the "hydration phase", then:
- no options passed: the instantiator creates the object via its constructor - hydrator fills in other properties which are not in the constructor
- withoutConstructor is passed: the instantiator creates an "empty shell" object - hydrator fills everything
- forceProperties / alwaysForceProperties / extraAttributes / allowExtraAttributes does not affect instantiation phase

[kbond]

Thinking Factory::hydrateWith() should work like this:

// customize the hydrator
public function initialize(): self
{
    return $this->hydrateWith(function(Hydrator $hydrator) {
        // $hydrator is the "default" hydrator service configured by bundle config
        // this allows us to inject property_accessor and property_info into it
        return $hydrator
            ->alwaysForceProperties()
            ->allowExtraAttributes()
        ; // clones the hydrator
    });

    // alternative to above
    return $this->hydrateWith(Hydrator::FORCE_PROPERTIES | Hydrator::ALLOW_EXTRA);

    // complete control
    return $this->hydrateWith(function(MyObject $object, array $remainingAttributes) {
        $object->someRichMethod($remainingAttributes['foo'], $remainingAttributes['bar']);

        return $object;
    });
}

[kbond]

And Factory::instantiateWith() could work something like:

public function initialize(): self
{
    return $this->instantiateWith(Instantiator::WITHOUT_CONSTRUCTOR);

    // or use a factory method
    return $this->instantiateWith([MyObject::class, 'factoryMethod']);

    // complete control:
    return $this->instantiateWith(function(array &$attributes) {
        $object = new MyObject($attributes['foo'], $attribtues['bar']);

        // required if not using a custom hydrator
        unset($attributes['foo'], $attributes['bar']);

        return $object;
    });
}

I think that's really the only custom config you'd need to make for the instantiator, or am I missing something?

In the bundle config, we'd only need boolean for the default instantiation mode: with constructor (default) or without.

[nikophil]

I like the bitwise configurations :)

for the instantiator:

• I also think this is the only config remaining in the instantiator
• it would be nice to not require a manual unset() in userland when using a custom instantiator, but I presently don't see how it could be done

for the hydrator: don't we also need forceProperties and extraAttributes?

[kbond]

it would be nice to not require a manual unset() in userland when using a custom instantiator, but I presently don't see how it could be done

Agreed, don't see a way around it currently either. If customizing both, you could do everything in the instantiator and somehow disable the hydrator.

for the hydrator: don't we also need forceProperties and extraAttributes?

Yep, these just couldn't be configured via bits

[nikophil]

@kbond I kinda remember that you said we were stuck here, but I'm not 100% sure? how would this integrate with Foundry v2 architecture?

[kbond]

I think this is at least partially solved. I think the remaining issue is with circular dependencies - two entities that require each other.

@1ed, sorry for the late response but is this issue still relevant?

[nikophil]

I'm closing this stale issue, feel free to reopen it, if it is still relevant