skip to Main Content

Most of my controllers in laravel look the same

Example:

class CourseController extends Controller
{
    public function index(Request $request)
    {
        $courses = Course::query();
        $courses = $this->commonIndex($courses, $request);
        return CourseResource::collection($courses);
    }
    public function store(StoreCourseRequest $request)
    {
        $course = Course::create($request->validated);
        return CourseResource::make($course);
    }
    public function show(Request $request, Course $course)
    {
        return CourseResource::make($course);
    }
    public function update(UpdateCourseRequest $request, Course $course)
    {
        $course->update($request->validated());
        return CourseResource::make($course);
    }
    public function destroy(Course $course)
    {
        $course->delete();
        return response()->noContent();
    }
}

I would like to create a common controller which is extended by all simple controllers and passes the Model, FormRequest, and Resource as public variables something like this.

class CommonController extends BaseController
{
    use AuthorizesRequests, ValidatesRequests;
    public $model;
    public $resource;
    public $storeFormRequest;
    public $updateFormRequest;
    public function index(Request $request)
    {
        $model = new $this->model;
        $resource = new $this->resource($request);
        $data = $model->query();
        return $resource->collection($data);
    }
    public function show(Request $request, string $id)
    {
        $model = new $this->model;
        $resource = new $this->resource($request);
        $data = $model->find($id);
        return $resource->make($data->load($loadWith));
    }
    public function store(Request $request)
    {
        $model = new $this->model;
        $resource = new $this->resource($request);
        $request = new $this->storeFormRequest($request->toArray());
        $validated = $request->validated();
        $data = $model->create($validated);
        return $resource->make($data);

    }
}

I don’t know what’s the best approach to passing the public variables to the constructor and evaluating them.

Also i’m facing an issue where new $this->storeFormRequest($request->toArray()) is evaluating to null.

The $storeFormRequest is the standard laravel FormRequest

class StoreCourseRequest extends FormRequest
{
    public function authorize(): bool
    {
        return true;
    }

    public function rules(): array
    {
        return [
            'name' => 'required|string',
            'description' => 'required|string',
            'language_id' => 'required|exists:languages,id',
            'image' => 'array',
        ];
    }
}

This is how i’m using the CommonController

class CourseController extends CommonController
{
    public $model = Course::class;
    public $resource = CourseResource::class;
    public $storeFormRequest = StoreCourseRequest::class;
    public $updateFormRequest = StoreCourseRequest::class;
}

2

Answers


  1. Overall, if your implementation of a base class controller is working how you want it, follow the rule of "if it ain’t broke, don’t fix it" 😉

    Having said that…

    I don’t know what’s the best approach to passing the public variables to the constructor and evaluating them.

    As you don’t need to access the member variables from outside the class you can make them protected; there’s no need for "setter" functions.

    class CommonController extends BaseController
    {
        use AuthorizesRequests, ValidatesRequests;
    
        protected $model;
        protected $resource;
        protected $storeFormRequest;
        protected $updateFormRequest;
    
        ...
    }
    
    class CourseController extends CommonController
    {
        protected $model = Course::class;
        protected $resource = CourseResource::class;
        protected $storeFormRequest = StoreCourseRequest::class;
        protected $updateFormRequest = StoreCourseRequest::class;
    }
    

    You can tidy up the CommonController methods a bit using static methods where they are available.

    As an example, instead of doing this:

        $model = new $this->model;
        ...
        $data = $model->find($id);    
    

    do this:

        $data = ($this->model)::find($id);
    
    Login or Signup to reply.
  2. If it were me, this is what I would do:
    First I would define CommonController as an abstract class, and subsequently add four abstract methods for getting the corresponding processing class name

    abstract protected function GetModel(): string;
    abstract protected function GetResource(): string;
    abstract protected function GetStoreFormRequest(): string;
    abstract protected function GetUpdateFormRequest(): string;
    

    Then I go and implement four methods in the subclass for returning the class name.

    protected function GetModel(): string
    {
        return Course::class;
    }
    

    Why?
    Using abstract methods constrains every subclass to implement them.
    This is just my personal opinion, I don’t think there is a right answer to this question

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search