7

I am developing a package for Laravel 5, and now I need to benefit from dependency injection to have a more scalable and relaible application, I don't know which approach is the best to take and why, this is a piece of my code and I need to injected the Lang class dependency

 class MyController extends \App\Http\Controllers\Controller
 {        
    public $text;
    public $lang;

    public function __construct()
    {         
       // Some codes here                            
    }

    public function myFunction(){
       $this->text = \Lang::get('package::all.text1');           
     }
}

In this link http://laravel.com/docs/4.2/ioc 2 approaches are suggested, Basic Usage and Automatic Resolution based on my understanding from the link taking the first approach I need to add

 App::bind('lang', function($app)
{
    return new \Lang();
 });    

to the register part of application and then in the function I'll have something like this :

 public function myFunction()
{
  $lang = \App::make('lang');       
  $this->text = $lang::get('package::all.text1');           
 }

The other way is to modify the constructor like

  public function __construct(Lang $lang)
  {         
      $this->lang = $lang;
  }

And then instantiate object from Class like

  $myController = App::make('MyController');

Which way is the better way to take for, considering that this class is a Controller and it will be called in the routes file, or please correct me if my understanding from the link is not right. please also inform me why you suggest any of those approaches.

Siavosh
  • 2,314
  • 4
  • 26
  • 50
  • 2
    I'd suggest taking it as a constructor argument unless you find yourself having lots of external dependencies (lang, request, uri, router, mail, etc.). Alternatively use the new [method dependency injection](http://laravel.com/docs/5.0/controllers#dependency-injection-and-controllers) (scroll down a little) if the method you're using is an actual controller action. – alexrussell Mar 03 '15 at 14:41
  • Also, your post appears to be missing the actual link you mention. – alexrussell Mar 03 '15 at 14:42
  • @alexrussell just edited the question and added the link – Siavosh Mar 03 '15 at 14:46
  • A-ha. Well first you're looking at the docs for 4.2. They're not seriously outdated for 5.0 applications, but the 5.0 documentation is pretty good so you should probably check that out. The 'IoC container' equivalent article is [here](http://laravel.com/docs/5.0/container). I'll write an answer with my personal thoughts for you, though. – alexrussell Mar 03 '15 at 14:53
  • @alexrussell thanks hadn't found the docs for laravel 5, so if I modify my `constuctor` do I have to change the whole `routes` file as well or not? for example `Route::get('/some/route', array('uses' => 'MyPackage\Namespace\ExampleController@index ));` will still work ? (ExampleController extends from MyController) – Siavosh Mar 03 '15 at 15:09
  • Nope - if your routes file does something like this: `Route::get('some/uri', '\My\Best\Controller@action');` then Laravel will create your controller using the IoC's constructor DI so you magically get things injected for you. Also remember that when you resolve a class from the IoC it's now an instance not a static facade - so `Lang::get()` becomes `$lang->get` or `$this->lang->get()` depending if it's a local or class instance. – alexrussell Mar 03 '15 at 15:12

1 Answers1

7

It should be noted that using local IoC resolution ($app->make() stylee) is not much better than using the facades directly (Lang::get() stylee) - you're still very much relying on Laravel's specific classes without really making your code explicitly state that it needs these exact classes. So the general advice is to, as much as possible, code to an interface if you want your code to be as portable as possible.

Of course there are a couple of big downsides to this currently in PHP development:

  1. These interfaces are not generally defined (except the PSR-3 LoggerInterface interface) so you still have to rely on a particular instance of the interface (in this case, Laravel's).
  2. If you decide to make your own generic interface (or the FIG eventually creates some of these), the classes that Laravel provides for translation (for example) don't implement it anyway, so you then need to subclass the existing ones just to make it look like it implements your own interface. But hey, that's the current best practice, so I guess if you wanna be using the current best practices, code to an interface, and don't worry for the time being that the interface you're coding to is Laravel-specific.

But anyway, here are my thoughts on your specific question. First off I should say that I haven't actually used Laravel 5 yet (just the 4s), but I have generally followed its development.

If the class I am coding will use a given dependency quite a lot or as a core part of how the class works I will use constructor dependency injection. Examples here are the Request or some Repository class in a controller, or a business logic class in a console command class.

If what I need I only need for a specific purpose (maybe redirecting from a controller and needing to generate a URI) I will resolve a local version from the IoC container ($this->app->make()) and then use that. If I were using Laravel 5 and the method was called by Laravel directly (e.g. a controller's action method) I may use method injection for this, I'm not 100% sure.

As a final note, the general advice is that if your constructor method signatures get too big due to a lot of dependencies:

  1. It's time to have a look at if your code relies too much on external dependencies. Maybe some of the functionality of your class can be extracted to its own class that splits the dependencies between the two.
  2. You should consider using setter methods rather than constructor injection - so instead of accepting a Request object, you have a $class->setRequest() method. The downside of doing this is that you need to tell Laravel's IoC container how to instantiate your object (i.e. that these setters must be called). It's not that big a deal but something worth noting.

Relevant links:

alexrussell
  • 13,856
  • 5
  • 38
  • 49
  • Thanks for your answer, since in many cases I have to use `static methods` and they can not be accessed with an instantiated class object, like the example in question: `\App::make('lang');` don't you think it would be better to apply the `($app->make() stylee)`, or in other words it seems mandatory – Siavosh Mar 04 '15 at 14:29
  • I'm not sure what you're asking there, but maybe it's worth me pointing out that calling the facades (like `Lang::get()`) is equivalent to resolving an (actually, *the*) instance out of the IoC container using `\App::make()` or `$app->make()` and then using it as an instance `$lang->get()`, as the facades in Laravel, behind the scenes, actually create an instance and act as proxy classes. So actually, I'd suggest you take a `Container` instance in your constructor so that the rest of the class can use `$this->app->make()` to resolve instances. – alexrussell Mar 04 '15 at 15:08
  • maybe I was not clear in my question but I meant this we can not have that as the attribute of the class, for example in class `MyClass` we can have `$lang->get()` but this lang can not be attribute of class and it can not be `$this->lang->get()` – Siavosh Mar 04 '15 at 15:34
  • You certainly can do it like that - either accept the `Container` or the `Translator` in your class's constructor and just set `$this->lang = $app->make('lang');` or `$this->lang = $lang;` It just depends on how much you're going to use the translator - if not a lot, it's best to store the `Container` instance (i.e. `App`/`$app`) in your class and resolve instances as you need them, if you'll be using it a lot in your class, by all means store the translator instance in your class. – alexrussell Mar 04 '15 at 17:01
  • I don't know if I get you right, OK I am using it a lot so your suggestion is to store it in class so far I agree, but then I can't call `static` methods of that, this is the issue unless it is injected via method – Siavosh Mar 04 '15 at 17:15
  • 1
    You've got it - use static calling if you're calling the facade (`Lang::`, `HTML::` `URL::`, `Router::`) but use instance methods if you have resolved an instance (`$this->translator->get()`). It's probably worth you reading up on how the Laravel facades work: http://laravel.com/docs/5.0/facades – alexrussell Mar 05 '15 at 09:07