DTOs in Controller Service Repository Pattern

Hi,

I have implemented controller service repository pattern in my application.

I have placed business logic in the services and am finding that I need to send arrays of information back to the controller, for example:

namespace App\Services

class ProductService
{

public function createProduct(array $attributes): array
{
    $organisation = $this->organisationRepository->getById($attributes['organisation_id']);

    if ($this->repository->getByOrganisation($organisation->id)->count() >= $organisation->maximum_products) {
        return [
            'success' => false,
            'data' => [],
            'message' => 'You have reached the maximum products for your account',
        ];
    }

    return [
        'success' => false,
        'data' => $this->repository->create($attributes),
        'message' => '',
        ];
    }
}

I am wondering if this is the best approach? I have looked into DTOs, would this be a good use case for a returning a standard object from the services?

Thanks for your advice

bugsysha

4 years ago

Level 61

Service classes are the worst thing that came out of the Symfony framework. They do not make any sense. It is just a bad habit that developers use when they can't identify the responsibility of the class. Just append "Service" at the end of the class name and it sounds better. But in reality, it is a major mess. The same applies when people append "er" at the end, like for Transformer.

Passing an array to the createProduct method is probably the place where you could benefit from DTO or an interface. Array doesn't guarantee that you have the required data.

You are lacking encapsulation. Also naming is confusing (bad), but I assume that it is caused by you trying not to give away too much info about the app.

martinbean

4 years ago

Level 80

Service classes are the worst thing that came out of the Symfony framework.

Service classes existed long before Symfony. They’re not Symfony-specific; they’re not even PHP-specific.

peterdickins

4 years ago

Level 2

Thank you for your reply @bugsysha , I was actually referring to the arrays being returned from the service to see if this would be benefit from a DTO?

bugsysha

4 years ago

Level 61

@peterdickins I know what you were referring to but you have bigger problems than that. But since you are insisting, I would also go with DTO for the return statement.

martinbean

4 years ago

Level 80

@peterdickins You need to think about the inputs and outputs of your service classes more. A service class shouldn’t be receiving arrays, nor returning arrays. An array says nothing about its contents. It doesn’t enforce keys, or their values or the types of any values.

If you have a service class method called createProduct, then it should receive the information it needs to create a product, and the result it returns should be a created Product entity instance (so that could be an Eloquent model or some other entity class in your application if you have one).

For the information you send to the method, this is where a DTO would come in. If you have your createProduct method then you might have a CreateProductAttributes class that actually enforces the attributes it should contain. This way, the DTO can only be constructed with valid data, and the service class method can only be called with a valid DTO instance. If your service class method is just accepting and returning arrays then that’s just a hotspot for bugs.

So, your method signature should look something like this:

public function createProduct(CreateProductAttributes $attributes): Product;

Reading that, I know I need to pass it an instance of CreateProductAttributes, and I’m going to get a Product instance back. I’m not reading it going, “It takes an array, and returns an array. OK, what does the attributes array need to contain? What does the array it return contain? Great, I’ve now got to open up that class and read that method to work out what it does…”

1 like

peterdickins

4 years ago

Level 2

Thank you @martinbean , yes that makes sense.

You say that the service method should return the created product, however, what if there is an error to return, like in the example above, or should the createdProductAttributes class have error fields?

martinbean

4 years ago

Best Answer

Level 80

what if there is an error to return, like in the example above, or should the createdProductAttributes class have error fields?

@peterdickins No. This is what exceptions are for.

If there’s an error executing domain logic, throw a domain exception. It’s then up to the caller to deal with these exceptions appropriately.

For example, a controller will probably want to convert an exception to some sort of HTTP responses, whereas a console command will want to surface the error differently (a HTTP response doesn’t make a lot of sense in a command line context).

Your service classes should not be context-aware. They shouldn’t be concerned with how they’re being called. Their job is to take some input and return a result, or indicate what went wrong by raising an exception. This is easy with Laravel’s exception handler as you can listen for exceptions there and return an appropriate response:

public function store(CreateProductRequest $request)
{
    // Create DTO here from request data...

    // This service class method may throw an exception...
    $product = $this->productService->createProduct($attributes);

    // Return response if product was successfully created
}

public function register()
{
    $this->renderable(function (CreateProductException $e, $request) {
        // If CreateProductException was raised, return back with exception message flashed to session
        return redirect()->back()->withInput()->withError($e->getMessage());
    });
}

This means your controllers (and other classes like console commands and queued jobs) aren’t stuffed full of horrible try / catch blocks.

Rendering exceptions documentation: https://laravel.com/docs/8.x/errors#rendering-exceptions

Please or to participate in this conversation.