thephpleague / factory-muffin

Enables the rapid creation of objects for testing
https://factory-muffin.thephpleague.com/
MIT License
531 stars 72 forks source link

[3.0] [Proposal] Bind FactoryMuffin to generator closures #351

Closed jonsa closed 9 years ago

jonsa commented 9 years ago

I use FactoryMuffin with Codeception and have abstracted the factory in my tester. In other words, I don't have access to the factory instance.

For various reasons I, at times, would like to create an instance of a Class2 as an attribute on Class1. In v2 of FactoryMuffin it was ok since all calls went through the facade but in v3 the facade does not exist anymore.

Let me show with an example.

// In v2 one could do something like this
$model = FactoryMuffin::create('Class1', [
    'foo' => function () {
        return FactoryMuffin::create('Class2');
    }
]);

// In v3 I would like to do something like this
$model = $I->persistClass('Class1', [
    'foo' => function () {
        /** @var FactoryMuffin $this **/
        return $this->create('Class2');
    }
]);

This could be achieved with a small modification to the CallableGenerator::__construct

public function __construct(callable $kind, $object, FactoryMuffin $factoryMuffin)
{
    if ($kind instanceof \Closure) {
        $kind = $kind->bindTo($factoryMuffin);
    }
    ...
}
GrahamCampbell commented 9 years ago

Interesting idea, but I'm not so sure. This may catch people by surprise.

The current way things work:

2.1 code:

$model = FactoryMuffin::create('Class1', [
    'foo' => function () {
        return FactoryMuffin::create('Class2');
    }
]);

3.0 code:

$model = $fm->create('Class1', [
    'foo' => function () use ($fm) {
        return $fm->create('Class2');
    }
]);

The way 3.0 currently works seems fine. It allows for maximum flexibility.

GrahamCampbell commented 9 years ago

Also, is there any reason why you're not using the factory generator here?

GrahamCampbell commented 9 years ago

Oh, yeh, it returns the id of the object, rather than the object. How about we change that behaviour in 3.0?

jonsa commented 9 years ago

The factory generator is still valid for relation properties so I'm not sure is should be changed. People might be more confused by that design change.

The reason I'm not using a use clause with my closure is that I don't have access to the factory instance within my test method. All I have access to is an "action" that I can give information to.

GrahamCampbell commented 9 years ago

Could you send a pull request with test please?

GrahamCampbell commented 9 years ago

Pull request merged.