constructor injection vs method injection

Question

Laravel encourages dependency injection. Since I am using laravel on my project, I figured I'll try using this approach.

I am taking advantage of Laravel's service container by type hinting my dependencies and letting it resolve them. I have four controllers. All of them extend a base class called GlobalController. I also have two models. All of them extend a base class called GlobalModel.

My first attempt is using method injection. GlobalController looks like this:

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Models\GlobalModel;

class GlobalController extends Controller
{

    public function __construct()
    {
        $this->middleware(['authenticate', 'token']);
    }

    // functions that handle normal http requests and ajax requests

    }

One of the controllers that extends from GlobalController is called UserController. Some of its functions are:

• index - show all data
• edit - show edit form
• update - updates to database

Edit and update use route-model-binding.

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Models\User;

class UserController extends GlobalController
{

    public function index(User $user)
    {
        $users = $user->all();
        return view('pages/view_users')->with('users', $users);
    }

    public function edit(User $user)
    {
        return view('pages/edit_user')->with('user', $user);
    }

    public function update(Request $request, User $user)
    {
        $data = $request->all();
        if ($user->validate($data))
        {
            $user->update($data);
            return $this->successResponse($request, 'users', 'Successfully edited user');
        }
        return $this->failedResponse($request, $user);
    }

    // other functions

    }

While this is working fine, Request and User get injected many times. If I have to change a Request implementation (for example), I'll have to manually change many functions to type hint that particular Request object. Not good at all. Since they are usually called in most functions, I tried doing constructor injection.

Here is GlobalController using constructor injection:

namespace App\Http\Controllers;

use Illuminate\Http\Request;
use App\Models\GlobalModel;

class GlobalController extends Controller
{
    protected $request;
    protected $model; // use polymorphism

    public function __construct(Request $request, GlobalModel $model)
    {
        $this->request = $request;
        $this->model = $model;
        $this->middleware(['authenticate', 'token']);
    }

    // functions that handle normal http requests and ajax requests

}

And here is UserController using constructor injection containing the same functions:

namespace App\Http\Controllers;

use Illuminate\Http\Request;;
use App\Models\User;

class UserController extends GlobalController
{

    public function __construct(Request $request, User $user) // use polymorphism
    {
        parent::__construct($request, $user);
    }

    public function index()
    {
        $users = $this->model->all();
        return view('pages/view_users')->with('users', $users);
    }

    public function edit(int $id)
    {
        $this->model = $this->model->find($id);
        return view('pages/edit_user')->with('user', $this->model);
    }

    public function update(int $id)
    {
        $this->model = $this->model->find($id);
        $data = $this->request->all();
        if ($this->model->validate($data))
        {
            $this->model->update($data);
            return $this->successResponse('users', 'Successfully edited user');
        }
        return $this->failedResponse();
    }

    // other functions

}

Now, I can't put my finger on it, but I think this implementation seems not right. It became less readable. The usage of $model and $this have made the code more disgusting.

I am so confused. I understand the benefits I can get from dependency injection, but I am sure my implementations of method injection and constructor injection are extremely wrong. What implementation should I choose? Or should I choose any from these two at all?

Answer 1

I definitely prefer the first method for Laravel Controllers. At first you don't need an injected model in every method. ( Why would you inject a user model in the index function?).

Second you cannot use the benefits of RouteModelBinding any longer and have to manually check whether a model with given $id really exists and take actions accordingly. Also you cannot use specific FormRequests like CreateUserRequest that could handle validation and authorisation. (This is an optional feature though)

Also note, that a model injected in constructor is never a 'real' model with user data. So this will just give you access to the eleoquent functions. So you could as well use User::find($id) in your code. This will always give you false.

public function __construct(User $user)
{
    dd($user->exists);
}

If you want to abstract things you could inject repositories in your constructor.

public function __construct(UserRepository $userRepository)
{
   $this->userRepository = $userRepository;
   // then the Repository is responsible for retrieving users
   // and you are not coupled to Eloquent. If you later want, you can Read
   // users from an XML File if you need
}

Additional info ( a bit offtopic ): Although it is very uncommon and I have never needed to change the request class you can do this by creating a custom request class like this:

namespace App;


use Illuminate\Http\Request;

class MyRequest extends Request
{
   // override request methods or add your new own methods
}

And then in the global index.php :

    $response = $kernel->handle(
    // instead of Illuminate\Http\Request::capture()
    $request = \App\MyRequest::capture()
);