chris_j's avatar

Designing a large project - specifically keep controllers short

This question is regarding design pattern(ish) and best practices in project design bearing in mind that SOLID design principles are goals and not rules. It's not Laravel specific but Jeffrey's series on SOLID principles encouraged me to ask here.

My team is rebuilding a project where in several instances a user request performs one action (that they know about) but carries out a chain or other operations (predominantly DB operations) as a consequence. Some of the latter operations may depend on what has happened before - usually to reference an item from the DB.

Overall, the whole operation is not that complex but I get the feeling there could be a better way of executing the process.

One controller in questions looks like this:

public function runProcess(ManagerX $managerX, ManagerY $managerY, etc ...)
{
        //...
        $valid = $validator->validateStuff();

        // maybe set some data required across the upcoming actions...

        $result1 = $managerX->doAction1();
        $result2 = $managerY->doAction2($result1);
	
        // etc...

        return $someKindOfResponse;
 }

The controller ends up being 70 lines long. I understand there are no hard and fast rules here but is there a more object orientated way of laying this out or is it just that large operations need to be written out this way?

I've heard of options such as service layers, using repositories etc but at the end of the day, each class/method needs to be called at some point and since some of the methods are re-used in other parts of the app, it seems sensible to keep them as de-coupled as possible.

Any observations or advice would be welcome.

0 likes
6 replies
chris_j's avatar

@giacholari thanks, yep I can see how that would work. There are approximately 4 Managers in the chain which makes me worry about how deep the rabbit hole goes. I suppose if they are all independent, discrete classes this should not matter?

Definitely agree with coding to an interface. There are several places where these operations (methods) are called so interfaces can definitely be made use of.

Thanks again for your input.

1 like
artcore's avatar

I'm enjoying command bus pattern. Reusable jobs keep the controller lean and you can queue jobs too, so some parts are async

two methods in my Document resource controller

public function index(Request $request)
  {
    if ($request->isXmlHttpRequest())
      return response()->json(['data' => $this->dispatchNow(new IndexDocumentJob($request->query()))]);
    
    return view('document::index');
  }


public function store(Request $request)
  {

//throws exception with error bags
    $this->dispatchNow(new ValidateDocumentJob($request));

//store the file
    $this->dispatchNow(new MoveDocumentJob($request));

//store in db
    $id = $this->dispatchNow(new StoreDocumentJob($request->input()));

//delete unlinked files  
    $this->dispatch(new StorageMaintenanceJob(new IndexDocumentJob()));//async
   
//json response or a redirect 
    return $this->dispatchNow(new RespondWithJsonOrRedirectJob($request,
        [
          'message'  => __('Successfully Stored!'),
          'id'       => $id,
          'redirect' => true,
        ]));
  }

The update method is similar

1 like
martinbean's avatar

@chris_j It’s impossible to give any specific advice without a specific example, but try to keep controllers as resourceful as possible. You’ll find you’ll probably unearth and discover new domain objects if you force yourself to use resourceful methods.

In terms of chains of operations, maybe look to adopt events and listeners, or run queued jobs in response to user actions.

1 like
chris_j's avatar

@martinbean Of course, the variety of scenarios and solutions is vast. I fully expect to have to refactor other parts of the application in response to changing the pattern.

I like the idea of injecting and 'chaining' off service providers from each other. When you understand they are still existing as discrete units it doesn't feel so tightly coupled.

@artcore Some of the operations may well be use cases for jobs/events.

The need to return to the controller each time a method finished just felt wrong somehow.

artcore's avatar

@chris_j for instance a form post will always have a few steps to go through. Validate, process and return a finished action; redirect or display a message. If you're more comfortable to have these steps in a dedicated class, why not, but you are adding unnecessary complexity imho. In my case the controller is still unaware and has no other responsibility than to start the jobs/commands. Take a look at ADR also - action domain response. This is probably the leanest way but adds complexity because of the sheer number of classes you'd have to deal with along with writing custom routes (Laravel uses controller actions by default)

Now that I think of it, I read this first on a blog by @martinbean :)

https://martinbean.dev/blog/2016/10/20/implementing-adr-in-laravel/ and he has a few more nice posts.

Please or to participate in this conversation.