Container::call() resolves array callable using class name #173

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants
@thispagecannotbefound
Contributor

thispagecannotbefound commented Jul 30, 2014

This change will allow you to call an array callable using a class name, like so:

$container->call(['MyClass', 'myMethod]);

If the method is not static, 'MyClass' will be resolved using the container.

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Jul 30, 2014

Member

This is very clever I love it! Good job on the tests, I'll merge it locally to remove the tabs ;)

Member

mnapoli commented Jul 30, 2014

This is very clever I love it! Good job on the tests, I'll merge it locally to remove the tabs ;)

@thispagecannotbefound

This comment has been minimized.

Show comment
Hide comment
@thispagecannotbefound

thispagecannotbefound Jul 30, 2014

Contributor

Thanks! Next addition could be to allow "MyClass::myMethod" for $callable, but IMO that brings more drawbacks than advantages, mainly because you lose the benefit of doing MyClass::class to get the FQCN. :)

Contributor

thispagecannotbefound commented Jul 30, 2014

Thanks! Next addition could be to allow "MyClass::myMethod" for $callable, but IMO that brings more drawbacks than advantages, mainly because you lose the benefit of doing MyClass::class to get the FQCN. :)

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Jul 30, 2014

Member

Totally agree with you.

Bad news though, there's a HHVM bug that makes the tests fail: https://travis-ci.org/mnapoli/PHP-DI/jobs/31234613 And since PHP-DI 4.1, HHVM is supported (i.e. tests must pass), so I'm afraid we'll need to wait for a fix in HHVM :(

Here is the bug: facebook/hhvm#3053 (I already reported it few weeks ago)

Member

mnapoli commented Jul 30, 2014

Totally agree with you.

Bad news though, there's a HHVM bug that makes the tests fail: https://travis-ci.org/mnapoli/PHP-DI/jobs/31234613 And since PHP-DI 4.1, HHVM is supported (i.e. tests must pass), so I'm afraid we'll need to wait for a fix in HHVM :(

Here is the bug: facebook/hhvm#3053 (I already reported it few weeks ago)

@mnapoli mnapoli added this to the 4.3 milestone Aug 4, 2014

@mnapoli mnapoli added the enhancement label Aug 4, 2014

mnapoli added a commit that referenced this pull request Aug 4, 2014

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 4, 2014

Member

OK sorry the bug with HHVM happens on master too, so it has nothing to do with your pull request :/

I have merged you PR manually and reverted your last commit (which wasn't related to this feature).

It's in the 4.3 branch: #174

Member

mnapoli commented Aug 4, 2014

OK sorry the bug with HHVM happens on master too, so it has nothing to do with your pull request :/

I have merged you PR manually and reverted your last commit (which wasn't related to this feature).

It's in the 4.3 branch: #174

@mnapoli mnapoli closed this Aug 4, 2014

@mnapoli mnapoli referenced this pull request Aug 4, 2014

Merged

4.3 #174

6 of 6 tasks complete
@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Aug 12, 2014

Member

I have just released PHP-DI 4.3.0 which includes this feature. Thanks!

http://php-di.org/news/11-php-di-4-3-released.html

Member

mnapoli commented Aug 12, 2014

I have just released PHP-DI 4.3.0 which includes this feature. Thanks!

http://php-di.org/news/11-php-di-4-3-released.html

@thispagecannotbefound

This comment has been minimized.

Show comment
Hide comment
@thispagecannotbefound

thispagecannotbefound Aug 13, 2014

Contributor

Great, thank you for the awesome library!

Contributor

thispagecannotbefound commented Aug 13, 2014

Great, thank you for the awesome library!

mnapoli added a commit that referenced this pull request Sep 27, 2014

@mnapoli

This comment has been minimized.

Show comment
Hide comment
@mnapoli

mnapoli Sep 27, 2014

Member

@thispagecannotbefound FYI I've pushed your idea onto callable classes too!

In v4.4 (not released yet):

$container->call('Foo');

// is the same as
$container->call(
    $container->get('Foo')
);

Have a look here: #192

Member

mnapoli commented Sep 27, 2014

@thispagecannotbefound FYI I've pushed your idea onto callable classes too!

In v4.4 (not released yet):

$container->call('Foo');

// is the same as
$container->call(
    $container->get('Foo')
);

Have a look here: #192

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