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

Dependency Injection in commands constructors #53

Merged
merged 2 commits into from
Nov 25, 2015
Merged

Dependency Injection in commands constructors #53

merged 2 commits into from
Nov 25, 2015

Conversation

antoniomadonna
Copy link

Use Laravel Container Class to resolve commands type hinted dependencies.

@jonnywilliamson
Copy link
Contributor

Just trying to follow this.

You want to use Laravel's IOC container so that you get automatic dependency injection when you create a class for a command?

So that

<?php

namespace App\Telegram\Bot\commands;

use App\Telegram\Bot\BotCommand;

class start extends BotCommand
{

    protected $demo;
    protected $example;

    public function __construct(Demo $demo, Example $example)
    {
        $this->demo = $demo;
        $this->example = $example;
    }

    protected $name = "start";

    protected $description = "start Description";

    public function handle($arguments)
    {
    }

}

Demo and Example are resolved for you automatically?

@antoniomadonna
Copy link
Author

Yes, exactly.

But I've just discovered that I forgot to check if the constructor is available. I'll fix asap.

Il giorno 11/nov/2015, alle ore 18:30, Jonathan notifications@github.com ha scritto:

Just trying to follow this.

You want to use Laravel's IOC container so that you get automatic dependency injection when you create a class for a command?

So that

demo = $demo; $this->example = $example; } protected $name = "start"; protected $description = "start Description"; public function handle($arguments) { } ``` } Demo and Example are resolved for you automatically? — Reply to this email directly or view it on GitHub.

@irazasyed
Copy link
Owner

Hey @antoniomadonna

Thanks for opening a PR. I appreciate your contribution.

I'll test this once you push a fix and merge it if i don't find any issues. Looks good to me. 👍

@antoniomadonna
Copy link
Author

ty @irazasyed

I've fixed the issue :)

@irazasyed
Copy link
Owner

@antoniomadonna Can you please rebase as per the latest changes in master branch?

Also, Please send a separate PR for each change (In this case, Tests should be in a separate PR). You have two different things pushed in one. Please update. Thanks!

Antonio added 2 commits November 25, 2015 15:31
Use Laravel Container Class to resolve commands type hinted dependencies.
@antoniomadonna
Copy link
Author

@irazasyed sorry I'm new to contributing, this PR should be ok now, I'm going to submit the tests asap

irazasyed added a commit that referenced this pull request Nov 25, 2015
Dependency Injection in commands constructors
@irazasyed irazasyed merged commit c33cb55 into irazasyed:master Nov 25, 2015
@irazasyed
Copy link
Owner

@antoniomadonna No problem. Thanks for separating the PRs and for all the contributions :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants