New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simpler closure definitions #226

Merged
merged 5 commits into from Jan 28, 2015

Conversation

Projects
None yet
4 participants
@mnapoli
Member

mnapoli commented Jan 27, 2015

Similarly to what Pimple did in v2 this PR makes declaring entries using closures simpler.

Before:

return [
    'foo' => DI\factory(function () {
        return new Foo();
    }),
];

After:

return [
    'foo' => function () {
        return new Foo();
    },
];

DI\factory() is now optional, closures will be automatically casted to factory definitions.

This only works with closures, DI\factory() still needs to be used with other callable types, e.g. invokable object, ['Doctrine\ORM\EntityManager', 'create'], …

If one wants to inject a callable (i.e. not execute it), DI\value() needs to be used:

return [
    'foo' => DI\value(function () {
        return new Foo();
    }),
    'bar' => DI\object('stdClass')
        ->constructor(DI\link('foo')),
];

This is more complex, but I don't think it's a problem as I've never ever seen that been used (same argument was used in Pimple).

@tsteur, @diosmosis thoughts?

@mnapoli mnapoli added the enhancement label Jan 27, 2015

@mnapoli mnapoli added this to the 5.0 milestone Jan 27, 2015

@mnapoli mnapoli self-assigned this Jan 27, 2015

@mnapoli mnapoli changed the title from Cast closure to factory to Simpler closure definitions Jan 27, 2015

@mnapoli mnapoli referenced this pull request Jan 27, 2015

Merged

v5.0 #200

31 of 31 tasks complete
@diosmosis

This comment has been minimized.

Show comment
Hide comment
@diosmosis

diosmosis Jan 27, 2015

I think specifying application logic in DI config (ie, injecting closures w/ logic) defeats the purpose of inversion of control, and I think in general defining callback properties is an anti-pattern, so I see no issues.

diosmosis commented Jan 27, 2015

I think specifying application logic in DI config (ie, injecting closures w/ logic) defeats the purpose of inversion of control, and I think in general defining callback properties is an anti-pattern, so I see no issues.

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Jan 27, 2015

Member

blahblahblah scratch all I've previously written here, I completely misunderstood your comment!

We are on the same page ;)

Member

mnapoli commented Jan 27, 2015

blahblahblah scratch all I've previously written here, I completely misunderstood your comment!

We are on the same page ;)

@avant1

This comment has been minimized.

Show comment
Hide comment
@avant1

avant1 Jan 27, 2015

Contributor

👍

Contributor

avant1 commented Jan 27, 2015

👍

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Jan 27, 2015

Contributor

👍

Contributor

tsteur commented Jan 27, 2015

👍

mnapoli added a commit that referenced this pull request Jan 28, 2015

@mnapoli mnapoli merged commit 4d9c4ab into 5.0 Jan 28, 2015

2 checks passed

Scrutinizer No new issues
Details
continuous-integration/travis-ci The Travis CI build passed
Details

@mnapoli mnapoli deleted the cast-closure-to-factory branch Jan 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment