I'm currently reading about "Lean controllers"/"Business logic on services" and trying to refactor a legacy code. However I am struggling to apply what is taught in the tutorials on real code.
Online examples are simple. But in real code, I have controllers that need to use filtering, sorting, filtering, sorting, pagination (and the fact that some of these elements are part of the Eloquent query makes me even more stuck):
Legacy code
class EmployeeController extends Controller
{
public function index(Request $request)
{
$employeesQuery = Employee::with(['gender', 'birthState', 'documentType', 'maritalStatus', 'addressState', 'user']);
//filters
$filters = ModelFilterHelpers::buildFilters($request, Employee::$acceptedFilters);
$employeesQuery = $employeesQuery->AcceptRequest(Employee::$acceptedFilters)->filter();
//sort
$employeesQuery = $employeesQuery->sortable(['updated_at' => 'desc']);
//get paginate and add querystring on paginate links
$employees = $employeesQuery->paginate(10);
$employees->withQueryString();
return view('employee.index', compact('employees', 'filters'))->with('i', (request()->input('page', 1) - 1) * 10);
}
Keeping everything together within just one service function feels wrong to me and separating into other functions seems complex to me.
What I have so far is:
App\Http\Controllers\EmployeeController.php
class EmployeeController extends Controller
{
public function index(Request $request)
{
//filters
$filters = ModelFilterHelpers::buildFilters($request, Employee::$acceptedFilters);
$employees = EmployeeService::getEmployees();
return view('employee.index', compact('employees', 'filters'))->with('i', (request()->input('page', 1) - 1) * 10);
}
App\Services\EmployeeService.php
class EmployeeService
{
public static function getEmployees() : LengthAwarePaginator
{
$employeesQuery = Employee::with(['gender', 'birthState', 'documentType', 'maritalStatus', 'addressState', 'user']);
$employeesQuery = $employeesQuery->AcceptRequest(Employee::$acceptedFilters)->filter();
//sort
$employeesQuery = $employeesQuery->sortable(['updated_at' => 'desc']);
//get paginate and add querystring on paginate links
$employees = $employeesQuery->paginate(10);
return $employees->withQueryString();
}
So I ask:
- Is this separation correct?
- Is it OK have this filtering, sorting, pagination in one service method?