thephpleague / factory-muffin

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

Better support for constructor only arguments for objects #345

Closed benglass closed 9 years ago

benglass commented 9 years ago

Nice library, I was experimenting with using it for a project but we have some objects that have required constructor arguments.

For example a Town object that has a required 'name' constructor argument.

Right now it seems the support for this is limited to using

FactoryMuffin::setCustomMaker(function($class) {
   if ($class == 'Town') {
      return new Town('name');
   }
});

Using the factory muffin to generate the constructor arguments isnt possible as far as I can see. It might be nice to be able to define a specific callback as part of FactoryMuffin::define that can be used to create your object and would receive all of the data generated for the object, or perhaps you could configure the constructor arguments separately like

class Town
{
    protected $name;
    public $otherProp;

    public function __construct($name)
    {
        $this->name = $name;
    }
}
FactoryMuffin::define('Town', array(
        'otherProp' => 'sentence|3'
    ), 
    null, // no callback in this case
    array('city') // array of definitions for constructor arguments in order
);

In this case factory muffin would generate data based on the last argument and pass it into the constructor when creating the class.

GrahamCampbell commented 9 years ago

Feel free to send a pull to the 3.0 version. Also, note that we've ditched the composer customisations in 3.0 because we've ditched the facade, so the recommended way to extend factory muffin is through actually extending the class now. :)

GrahamCampbell commented 9 years ago

@benglass How would using the illuminate container component to instantiate classes sound? Or would that be massively overkill?

benglass commented 9 years ago

I think that might be overkill and would also couple the library to a specific DI container which could be unappealing to users who are either unfamiliar with DI or are using a different DI container.

In our particular use case we were generating classes that require other classes in their constructor, we wanted the fixture library to be able to randomly generate a County and inject it into the constructor of a randomly generated Town.

We ended up using nelmio/alice which has a very robust expression language approach to fixture generation as opposed to a code-based approach that factory muffin uses. I dont think one is superior to another (some people don't like yml) but it did support a lot of features which factory muffin didnt at the time.

https://github.com/nelmio/alice#specifying-constructor-arguments

GrahamCampbell commented 9 years ago

See the progress on https://github.com/thephpleague/factory-muffin/pull/355.

GrahamCampbell commented 9 years ago

I've decided not to actually implement custom constructor arguments for now because that would require the use of reflection, and probably isn't useful to most people using this library.

benglass commented 9 years ago

You may want to consider a label such as wontfix since the issue is not actually resolved.

GrahamCampbell commented 9 years ago

I'm still not totally done here. I am going to propose another idea tomorrow.

GrahamCampbell commented 9 years ago

@benglass Ok, here we go:

In Factory Muffin 2.1, it was possible to register custom makers/savers/deleters, but they were registered globally through the facade.

In Factory Muffin 3.0, I've rewritten the architecture to be much better. We now have no facade, so the user is responsible for keeping track of any factory muffin instance they create, which is better, because they can deal with multiple ORMs etc.

In Factory Muffin 3.0, the ability to register custom makers/setters/deleters on the factory muffin class has been removed, and as you pointed out, most code to use these methods would be hacky, and based on checking the class of the model involved, so was not a good idea for larger code bases.

In Factory Muffin 3.0, the model definition registration has been improved massively fixing a few design problems we had before, and improving the definition group functionality. After this refactoring, I now propose that we allow people to register custom makers during this definition registration process, so in the same way you can define attribute definitions and callback for your models, you will also be able to register a custom maker closure.

Ok, so what does this mean for my code then?

Factory Muffin 2.1:

FactoryMuffin::define('Foo', []);
FactoryMuffin::define('extra:Foo', ['user' => 'firstNameMale']);
FactoryMuffin::define('Bar', []);
FactoryMuffin::define('Baz', []);

// no way to handle makers custom to groups
// we have to do this all at once - not very flexible
FactoryMuffin::setCustomMaker(function($class) {
    if ($class === 'Foo') {
        return new $class('bar');
    } elseif ($class === 'Bar') {
        return new $class('baz');
    } else {
        return new $class();
    }
});

Factory Muffin 3.0:

$fm = new FactoryMuffin();

$fm->define('Foo')
    ->setMaker(function ($class) {
        return new $class('bar');
});

$fm->define('extra:Foo')
    ->addDefinition('user', Faker::firstNameMale())
    ->setMaker(function ($class) {
        return new $class('hello');
});

$fm->define('Bar')
    ->setMaker(function ($class) {
        return new $class('baz');
});

$fm->define('Baz');

See the pull request here: https://github.com/thephpleague/factory-muffin/pull/357