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
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…
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.You can tidy up the
CommonController
methods a bit using static methods where they are available.As an example, instead of doing this:
do this:
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 nameThen I go and implement four methods in the subclass for returning the class name.
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