Skip to content
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

Support for invokable objects in Container::call() #185

Closed
andersonamuller opened this issue Aug 17, 2014 · 8 comments
Closed

Support for invokable objects in Container::call() #185

andersonamuller opened this issue Aug 17, 2014 · 8 comments
Labels
Milestone

Comments

@andersonamuller
Copy link

@andersonamuller andersonamuller commented Aug 17, 2014

I know this is future compatibility feature (PHP 5.4), but I cannot use a object that implements the method __invoke in the container->call method. Now it throws an exception.

Example:

class SendMailAction 
{
    private $mailer;

    public function __construct(MailerInterface $mailer) 
    {
        $this->mailer = $mailer;
    }

    public function __invoke($sender, $recipient, $subject, $content) 
    {
        $message = $this->mailer->compose($sender, $recipient, $subject, $content);
        return $this->mailer->send($message);
    }
}

$container->call(
    $container->make(SendMailAction::class),
    [
        'sender' => 'sender@example.com',
        'recipient' => 'recipient@example.com',
        'subject' => 'Testing',
        'content' => 'This is a test message'
    ]
);
@mnapoli mnapoli added the enhancement label Aug 17, 2014
@mnapoli
Copy link
Member

@mnapoli mnapoli commented Aug 17, 2014

Oh right that's a good idea to add support for that. You could consider it a missing feature :p

Let's keep this issue open, hopefully this could be done for the next minor version. If you have time to work on this let me know, else I'll have a look as soon as possible (it shouldn't be a lot of work).

@mnapoli
Copy link
Member

@mnapoli mnapoli commented Aug 17, 2014

By the way I love the look of that piece of code:

$container->call(
    $container->make(SendMailAction::class),
    [
        'sender' => 'sender@example.com',
        'recipient' => 'recipient@example.com',
        'subject' => 'Testing',
        'content' => 'This is a test message'
    ]
);

Sleek ;)

@andersonamuller
Copy link
Author

@andersonamuller andersonamuller commented Aug 17, 2014

Thanks, and I love the features of this package :)

I have an idea of where and how to implement it, but I just started working with PHP-DI, and I may miss some internal working. I don't have much time now to test it, that's why I didn't submit a PR together with the issue.

@mnapoli
Copy link
Member

@mnapoli mnapoli commented Aug 17, 2014

Sure no problem, I'll mark it for the next version and I'll look into it, like I said it should be fairly quick.

@mnapoli mnapoli added this to the 4.4 milestone Aug 17, 2014
mnapoli added a commit that referenced this issue Aug 17, 2014
Callable objects are objects that implement an __invoke() method.
mnapoli added a commit that referenced this issue Aug 17, 2014
@mnapoli
Copy link
Member

@mnapoli mnapoli commented Aug 17, 2014

OK I implemented this and put it in a 4.4 branch: #187

@mnapoli mnapoli closed this Aug 17, 2014
@mnapoli mnapoli mentioned this issue Aug 17, 2014
Merged
7 of 7 tasks complete
@andersonamuller
Copy link
Author

@andersonamuller andersonamuller commented Aug 17, 2014

Well done, I wouldn't have done such extensive re-factoring.

@mnapoli mnapoli changed the title Support for invokable objects Support for invokable objects in Container::call() Sep 1, 2014
mnapoli added a commit that referenced this issue Sep 27, 2014
@mnapoli
Copy link
Member

@mnapoli mnapoli commented Sep 27, 2014

@andersonamuller Have a look at #192, I've extended the support for callable objects:

$container->call('Foo');

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

It will be included in v4.4 too.

@andersonamuller
Copy link
Author

@andersonamuller andersonamuller commented Sep 27, 2014

👍 very nice, thanks

mnapoli added a commit that referenced this issue Sep 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.