I have read a number of sources that hint that laravel facade's ultimately exist for convenience and that these classes should instead be injected to allow loose coupling. Even Taylor Otwell has a post explaining how to do this. It seems I am not the only one to wonder this.
use Redirect;
class Example class
{
public function example()
{
return Redirect::route("route.name");
}
}
would become
use Illuminate\Routing\Redirector as Redirect;
class Example class
{
protected $redirect;
public function __constructor(Redirect $redirect)
{
$this->redirect = $redirect
}
public function example()
{
return $this->redirect->route("route.name");
}
}
This is fine except that I am starting to find that some constructors and methods are beginning to take four+ parameters.
Since the Laravel IoC seems to only inject into class constructors and certain methods (controllers), even when I have fairly lean functions and classes, I am finding that constructors of the classes are becoming packed out with the needed classes that then get injected into the needed methods.
Now I am finding that if I continue down this approach that I will need my own IoC container, which feels like reinventing the wheel if I am using a framework like laravel?
For example I use services to control the business / view logic rather than controllers dealing with them - they simply route the views. So a controller will first take its corresponding service
, then then the parameter
in its url. One service function also needs to check the values from a form, so then I need Request
and Validator
. Just like that, I have four parameters.
// MyServiceInterface is binded using the laravel container
use Interfaces\MyServiceInterface;
use Illuminate\Http\Request;
use Illuminate\Validation\Factory as Validator;
...
public function exampleController(MyServiceInterface $my_service, Request $request, Validator $validator, $user_id)
{
// Call some method in the service to do complex validation
$validation = $my_service->doValidation($request, $validator);
// Also return the view information
$viewinfo = $my_service->getViewInfo($user_id);
if ($validation === 'ok') {
return view("some_view", ['view_info'=>$viewinfo]);
} else {
return view("another_view", ['view_info'=>$viewinfo]);
}
}
This is a single example. In reality, many of my constructors already have multiple classes being injected (Models, Services, Parameters, Facades). I have started to 'offload' the constructor injection (when applicable) to method injection, and have the classes calling those methods use their constructors to inject dependencies instead.
I have been told that more than four parameters for a method or class constructor as a rule of thumb is bad practice / code smell. However I cannot see how you can really avoid this if you choose the path of injecting laravel facades.
Have I got this idea wrong? Are my classes / functions not lean enough? Am I missing the point of laravels container or do I really need to think of creating my own IoC container? Some others answers seems to hint at the laravel container being able to eliminate my issue?
That said, there doesn't seem to be a definitive consensus on the issue...
This is one of the benefits of constructor injection - it becomes obvious when you class is doing to much, because the constructor parameters grow too large.
1st thing to do is split up controllers that have too many responsibilities.
Say you have a page controller:
Class PageController
{
public function __construct(
Request $request,
ClientRepositoryInterface $clientrepo,
StaffRepositortInterface $staffRepo
)
{
$this->clientRepository = $clientRepo;
//etc etc
}
public function aboutAction()
{
$teamMembers = $this->staffRepository->getAll();
//render view
}
public function allClientsAction()
{
$clients = $this->clientRepository->getAll();
//render view
}
public function addClientAction(Request $request, Validator $validator)
{
$this->clientRepository->createFromArray($request->all() $validator);
//do stuff
}
}
This is a prime candidate for splitting into two controllers, ClientController
and AboutController
.
Once you have done that, if you still have too many* dependencies, its time to look for what i will call indirect dependancies (because i cant think of the proper name for them!) - dependencies that are not directly used by the dependant class, but instead passed on to another dependency.
An example of this is addClientAction
- it requires a request and a validator, just to pass them to the clientRepostory
.
We can re factor by creating a new class specifically for creating clients from requests, thus reducing our dependencies, and simplifying both the controller and the repository:
//think of a better name!
Class ClientCreator
{
public function __construct(Request $request, validator $validator){}
public function getClient(){}
public function isValid(){}
public function getErrors(){}
}
Our method now becomes:
public function addClientAction(ClientCreator $creator)
{
if($creator->isValid()){
$this->clientRepository->add($creator->getClient());
}else{
//handle errors
}
}
There is no hard and fast rule as to what number of dependencies are too many. The good news is if you have built your app using loose-coupling, re-factoring is relatively simple.
I would much much rather see a constructor with 6 or 7 dependencies than a parameterless one and a bunch of static calls hidden throughout the methods