Marco Aurelio's avatar

Lean controllers/business logic on services and filtering, sorting, pagination methods on controllers

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?
0 likes
5 replies
bugsysha's avatar

(using services)

There is your mistake. Services are the worst thing that came out of the Symfony framework. It is just a convenient way to hide unorganized code under the rug. Then it keeps growing until it becomes unmaintainable and in that process, it is the first place where SRP is broken.

First I would advise that you focus on making your code follow community standards. Mixing multiple naming conventions is always going to make your code look ugly. Bad formatting also doesn't help. Your code is a very good example where trying to force clarity doesn't work.

I'm not sure what is the tutorial that you are following, but either you are missing the point of the tutorial, or the tutorial is bad. But in both scenarios, it feels like you need to return to more basic stuff before you try to tackle event the simplest principles like SRP. Don't be discouraged with this. I was there, and I'm pretty sure that even the best developers from this forum were at one point.

martinbean's avatar

There is your mistake. Services are the worst thing that came out of the Symfony framework.

@bugsysha Why do you keep saying this? “Services” has nothing to do with Symfony. Services exist in other languages and frameworks, and I dare say were around before Symfony was even a thing.

bugsysha's avatar

@martinbean that is just an assumption where OP got this habit from or the person in the tutorial. I never claimed that it originated from Symfony. Shortest path. The simplest explanation is usually the correct one.

Still, it doesn't change the point. Those classes are dangerous if not used wisely.

martinbean's avatar

I never claimed that it originated from Symfony.

Services are the worst thing that came out of the Symfony framework.

bugsysha's avatar

@martinbean claiming something originated from Symfony and saying Symfony made it popular in PHP is totally different in my mind. You've made the wrong assumption about what "came out" mean(s) Mr. Bean.

S in parenthesis is just to highlight rhyme of mean and Bean :)

Please or to participate in this conversation.