luismilanese's avatar

Advice needed: how to make thinner controllers by removing validations and other decisions from it?

Hi everyone,

I'd like to get some hints from you how can I manage such situation.

Let's say I have a form from which the user can upload an image. For such task, we could retrieve the image's information with:

public function store(Request $request)
{
    $variable = $request->file('product-imagem');
    // Method goes on
}

But in my situation, I have a number of actions to perform with that file. This is an hipotetical example, no real situation here. An example of code could be:

public function store(Request $request)
{
    $image = $request->file('product-imagem');

    $resize = $image->resize(x, y);

    if (!$resize->isValid()) {
        throw new ResizeInvalidException(...);
    }

    $image->sendImageToAnExternalService();

    $image->doThis();
    $image->doThat();

    // After several things with just the image uploaded, I'll finally save the form
    Example::create($request->all());    

    return redirect("/some-location");
}

You see, there's way too much logic in that controller. One option I see is: create a method on the controller to place all that image logic (previous to Example::create()), but then it looks like I'm just moving the "dirt" around. And it doesn't seem to be a case for a service. Where could I place all this code?

This whole "image handling" is just an example. The wider question would be: if I have to treat data from a form, for example, before I save it, where is it likely to be all this code?

I hope my question is clear. Any help is appreciated.

0 likes
4 replies
ohffs's avatar
ohffs
Best Answer
Level 50

There are lots of ways to do this - and it'll be different depending on your code and the logic/flow/size of your application. For instance you could create a 'class per form' which knows how to process the data from each form. In turn there might be shared logic between those which you could extract to their own classes (eg, maybe you have multiple forms that upload & resize an image). But maybe that doesn't suit your application/logic so you might be better with a different pattern or using shared 'traits' or.... ;-)

This video has some nice refactoring of a controller method into smaller classes if that's the route you take :-)

luismilanese's avatar

Hi @ohffs, First off, thank you for your reply. I've bookmarked that video and I'm definitely watching it when I have a free time to really put into and absorb most of it.

As for that "class per form" approach, would you have an exame of it? I don't want to take any further time from you, but if you do have an example in hands of how would all that work together, I'd really appreciate it.

Many thanks.

ohffs's avatar

Say you have a form to create a new product, you could have :

app/FormHandlers/CreateProduct.php
class CreateProduct {
  public function __construct($newProduct, $formData) {
    $this->product = $newProduct;
    $this->formData = $formData;
  }

  public function process() {
     // do whatever logic for the formData and set $this->product fields
     return $this->product; // or save it here if that makes more sense
  }
}

And so on for each form. Then in your controller you could do something like :

// ProductCreateRequest is a Form Request from 'php artisan make:request' to handle basic validation, see
// https://laravel.com/docs/5.2/validation#form-request-validation
public function store(ProductCreateRequest $request) {
  $processor = new CreateProduct(new Product, $request->all());
  $product = $processor->process();
  return // whatever
}

public function update(ProductUpdateRequest $request, $id) {
  $processor = new UpdateProduct(Product::findOrFail($id), $request->all());
  $product = $processor->process();
  ...
}

You could also look into repository patterns .

luismilanese's avatar

@ohffs once again, thank you very much for your time and help.

Have a nice 2016. Best regards.

Please or to participate in this conversation.