What, I had been doing previously was to inject only MY MODELS using the constructor and use Facades for the Laravel's provided classes i.e. Session
, Auth
, Validator
etc, for example. Will it be a good idea if I inject each and every class (either mine or Laravel's) through construct and use it by $this->..
syntax or should I inject my own classes using constructor and use Facades for anything provided by Laravel?
To be more specific, here is what my controllers normally look like:
class MyController extends BaseController
{
public function __construct( User $user, Bookmark $bookmark ) {
$this->user = $user;
$this->bookmark = $bookmark
}
public function foobar ( ) {
$user_id = Input::get('bar');
...
Session::get('someInfo');
...
return Redirect::to('/');
}
...
}
Should I structure my methods like controller like following, instead?
class MyController extends BaseController
{
public function __construct( User $user, Bookmark $bookmark, Input $input, Session $session, Redirect $redirect ) {
$this->user = $user;
$this->bookmark = $bookmark
$this->input = $input;
$this->session = $session;
$this->redirect = $redirect;
}
public function foobar ( ) {
$user_id = $this->input->get('bar');
...
$this->session->get('someInfo');
...
return $this->redirect->to('/');
}
...
}
Laravel now supports the same injection functionality for methods of classes (not just constructors) that are route-related, such as controllers and middleware.
You could prevent unnecessary injections by only injecting to methods where the dependency is unique, perhaps leaving more common dependencies in the constructor:
class MyController extends BaseController
{
public function __construct( Input $input, Session $session, Redirect $redirect ) {
$this->input = $input;
$this->session = $session;
$this->redirect = $redirect;
}
public function foobar ( User $user, Bookmark $bookmark ) {
$user_id = $this->input->get('bar');
...
$this->session->get('someInfo');
...
return $this->redirect->to('/');
}
...
}
As for whether you should do it this way, that's up to you - forcing all dependencies to appear in method definitions seems cleaner to me, and easier to unit test.
It is elegant and useful to inject certain classes, such as Request. In my opinion they should be specified in controller methods where they are needed, as they are then logically connected to the method implementation. Awesome thus far.
I find two facades to be problemmatic - App and Log. Neither are logically connected to a controller or its actions. App and Log are not inputs in any context. As App and Log are utility classes they are relevant to services and repositories as well, and it gets downright nasty if you type hint them in controllers and then pass them on as constructor or method parameters to your support classes.
An additional issue is that App facade does not implement the Illuminate\Contracts\Auth\Guard interface that it proxies, so my IDE lights up with warnings as static analysis is not possible.
For the sake of consistency and overall separation of concerns I would therefore instantiate both App and Log within a constructor or method, depending on how widespread they are used in a class. To make my IDE happy I created the below class to give me a properly typed instance wherever I need it:
<?php namespace App\Components;
use Illuminate\Contracts\Auth\Guard;
use Psr\Log\LoggerInterface;
/**
* Get the underlying object instances of facades from the container.
*/
class AppGlobal
{
/**
* Returns the global logger instance.
*
* @return LoggerInterface
*/
public static function log()
{
return app('log');
}
/**
* Returns the global auth instance, which internally proxies a guard.
*
* @return Guard
*/
public static function auth()
{
return app('auth');
}
}
If you need an object wit properties - put it in as an injection (e.g Input, Session...), otherwise, if you don't store any data in the object and pretty happy using class, than go with facades (e.g Log::..., Redirect::...).
Laravel has replaced many of it's facade with helpers for example
use Auth;
and
Auth::user()
is now just
auth()->user()
this makes thing simpler and neater (also prevents mistakes)
I would suggest using the helpers where possible and if no helper exists, use the facade because it is easier to mock than an injected instance.