Using dependency injection over laravel facades
Asked Answered
H

6

50

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...

Honor answered 26/1, 2016 at 10:10 Comment(5)
Best practices are always guidelines, not set in stone rules. Some patterns conflict entirely, and therefore you have to weigh the pros and cons of each and make a judgement call. Personally, when it comes to the constructor a lot of injected dependencies may be inevitable. But it's also not something you use all the time outside of testing within Laravel. The IoC handles them for you, so don't get too afraid of loading up your constructors.Enjoy
To correct myself, when I said "not something you use all the time" I mean you aren't going to be manually providing the dependencies for your constructors. Since the IoC will handle providing them, the problems that result from overstuffed methods isn't really the same. Still, I'd personally keep it to the constructors, not the specific methods.Enjoy
So you don't think constructors (in central places) with lots of classes being injected is an issue?Honor
Why not just use the redirect() helper function? return redirect('something');Braiding
@Honor - No I don't. I think it might be evidence of a problem, as was said in another answer but sometimes there may be a case where a principle does not fit your use case. What you want to avoid is over-engineering your software, as well. There's a fine line between writing readable, manageable code and being so locked into a design principle that you spend hours upon hours trying to force code into it when it doesn't make sense.Enjoy
P
30

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

Penal answered 5/2, 2016 at 0:19 Comment(4)
I have re-factored my classes so that the 'indirect' laravel dependencies are now injected into these SRP classes (validation, formatting, requests) etc but I still have one main business logic 'service' which I inject all these classes into to then use. I think your answer hints at a pattern issue I am missing..Honor
If you could update your question to show how you use this service, i would be happy to take a look. At a guess, you have probably gone from one extreme to the other - taking all the logic out of the controller, and instead made your single 'business service' into a god class. Probably the business service needs to be broken down via a similar processPenal
after much refactoring, it was indeed a case of classes having 'too much responsibility'. I learnt a valuable lesson and now my project is much more resilient to change.Honor
@Honor Great, glad it worked out for you, and thanks for coming back to report the outcomePenal
R
3

One issue with facades is that additional code has to be written to support them when doing automated unit testing.

As for solutions:

1. Resolving dependencies manually

One way of resolving dependencies, if you do not wish to do it via. constructors or methods injection, is to call app() directly:

/* @var $email_services App\Contracts\EmailServicesContract
$email_services = app('App\Contracts\EmailServicesContract');

2. Refactoring

Sometimes when I find myself passing too many services, or dependencies into a class, maybe I have violated the Single Responsibility Principe. In those cases, maybe a re-design is needed, by breaking the service or dependency into smaller classes. I would use another service to wrap up a related group of classes to serve something as a facade. In essence, it'll be a hierarchy of services/logic classes.

Example: I have a service that generate recommended products and send it out to users via email. I call the service WeeklyRecommendationServices, and it takes in 2 other services as dependency - a Recommendation services which is a black-box for generating the recommendations (and it has its own dependencies -- perhaps a repo for products, a helper or two), and an EmailService which maybe has Mailchimp as a dependency). Some lower-level dependencies, such as redirects, validators, etc. will be in those child services instead of the service that acts as the entry point.

3. Use Laravel global functions

Some of the Facades are available as function calls in Laravel 5. For instance, you can use redirect()->back() instead of Redirect::back(), as well as view('some_blade) instead of View::make('some_blade'). I believe it's the same for dispatch and some other commonly used facades.

(Edited to Add) 4. Using traits As I was working on queued jobs today, I also observe that another way to inject dependencies is by using traits. For instance, the DispathcesJobs trait in Laravel has the following lines:

   protected function dispatch($job)
    {
        return app('Illuminate\Contracts\Bus\Dispatcher')->dispatch($job);
    }

Any class that uses the traits will have access to the protected method, and access to the dependency. It's neater than having many dependencies in the constructor or method signatures, is clearer (about what dependencies are involved) than globals and easier to customize than manual DI container calls. The drawback is that each time you invoke the function you have to retrieve the dependency from the DI container,

Responsible answered 4/2, 2016 at 19:19 Comment(2)
Why would you recommend function calls over explicitly injecting the needed classes? Could this not lead to hard to spot dependencies?Honor
1. If the dependencies are standard dependencies (such as redirect, dispatch etc), I think it's acceptable. 2. Those global functions are actually calls to the DI resolver.Responsible
B
1

Class methods that form a part of the routing mechanism in Laravel (middleware, controllers, etc.) also have their type-hints used to inject dependencies - they don't all need to be injected in the constructor. This may help to keep your constructor slim, even though I'm not familiar with any four parameter limit rule of thumb; PSR-2 allows for the method definition to be stretched over multiple lines presumably because it's not uncommon to require more than four parameters.

In your example you could inject the Request and Validator services in the constructor as a compromise, since they're often used by more than one method.

As for establishing a consensus - Laravel would have to be more opinionated for applications to be similar enough to utilise a one-size-fits-all approach. An easier call though is that I think facades will go the way of the dodo in a future version.

Bolme answered 27/1, 2016 at 0:45 Comment(0)
H
1

Not so much an answer but some food for thought after talking to my colleagues who have made some very valid points;

  1. If the internal structure of laravel is changed between versions (which has happened in the past apparently), injecting the resolved facade class paths would break everything on an upgrade - while using the default facades and helper methods mostly (if not completely) avoids this issue.

  2. Although decoupling code is generally a good thing, the overhead of injecting these resolved facade class paths makes classes cluttered - For developers taking over the project, more time is spent trying to follow the code which could be spent better on fixing bugs or testing. New developers have to remember which injected classes are a developers and which are laravels. Developers unfamiliar with laravel under the hood have to spend time looking up the API. Ultimately the likelihood of introducing bugs or missing key functionality increases.

  3. Development is slowed and testability isn't really improved since facades are already testable. Rapid development is a strong-point of using laravel in the first place. Time is always a constraint.

  4. Most of the other projects use laravel facades. Most people with experience using laravel use facades. Creating a project that doesn't follow the existing trends of previous projects slows things down in general. Future inexperienced (or lazy!) developers may ignore facade injection and the project may end up with a mixed format. (Even code reviewers are human)

Honor answered 29/1, 2016 at 10:23 Comment(0)
S
1

Well your thoughts and concerns and correct and I had them as well. There are some benefits of Facades ( I generally dont use them ), but if you do use just I would suggest using them only in the controllers, as the controllers are just entry and exit points for me at least.

For the example you gave I'll show how I generally handle it:

// MyServiceInterface is binded using the laravel container
use Interfaces\MyServiceInterface;
use Illuminate\Http\Request;
use Illuminate\Validation\Factory as Validator;

...
class ExampleController {

    protected $request;

    public function __constructor(Request $request) {
        // Do this if all/most your methods need the Request
        $this->request = $request;
    }

    public function exampleController(MyServiceInterface $my_service, Validator $validator, $user_id) 
    { 
        // I do my validation inside the service I use,
        // the controller for me is just a funnel for sending the data
        // and returning response

        //now I call the service, that handle the "business"
        //he makes validation and fails if data is not valid
        //or continues to return the result

        try {
            $viewinfo = $my_service->getViewInfo($user_id);
            return view("some_view", ['view_info'=>$viewinfo]);
        } catch (ValidationException $ex) {
            return view("another_view", ['view_info'=>$viewinfo]);
        }
    }
}



class MyService implements MyServiceInterface {

    protected $validator;

    public function __constructor(Validator $validator) {
        $this->validator = $validator;
    }

    public function getViewInfo($user_id, $data) 
    { 

        $this->validator->validate($data, $rules);
        if  ($this->validator->fails()) {
            //this is not the exact syntax, but the idea is to throw an exception
            //with the errors inside
            throw new ValidationException($this->validator);
        }

        echo "doing stuff here with $data";
        return "magic";
    }
}

Just remember to break your code to small individual pieces that each one handles his own responsibility. When you properly break your code, in most cases you will not have so many constructor parameters, and code will be easily testable and mocked.

Just one last note, if you are building a small application or even a page in a huge application for example a "contact page" and "contact page submit", you can surely do everything in the controller with facades, it simply depends on the complexity of the project.

Scandian answered 3/2, 2016 at 7:35 Comment(3)
I understand what you are saying, but if you have a single point doing the business logic (like a service), even if you create separate services for validation, formatting, etc etc the injected laravel classes are simply replaced by your own injected classes (while injecting the laravel classes into those classes)Honor
just in the example I posted you could see that the dependencies were divided... Just don't overuse it as for refactoring each action to a class. Make sure you make your code "SOLID" and in 90% of the cases you wont have so many dependenciesScandian
You can surely give me an example in the post, and I'll try to refactor it.Scandian
T
1

I love the laravel due to its beautiful architecture.Now as from my approach i wouldnt inject all the facades in to the controller method only why? Injecting Redirect facades only in controller wrong practices as it might need in other. And mainly the things that are mostly used should be declared for all while for those who uses some or only then its best practice to inject them via method as when you declare at top it will hamper in your memory optimization as well as the speed of your code. Hope this would help

Tremulous answered 4/2, 2016 at 10:26 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.