Single Responsibility: Move Controller Code to Trait

In this lesson, we will discuss SOLID code with the Single Responsibility Principle and clearer method names.

The Problem: Base Controller and Trait

First, the project has a "Base" API Controller with a few methods that can be reused in other Controllers. Some of them are for typical API responses.

app/Http/Controllers/Api/V1/ApiController.php

class ApiController extends Controller
 
    // ...
 
    public function notAuthorized(string $message = 'You are not authorized.')
    {
        return $this->error($message, Response::HTTP_UNAUTHORIZED);
    }
 
    public function notFound(string $message = 'Not Found.')
    {
        return $this->error($message, Response::HTTP_NOT_FOUND);
    }
 
    public function unexpectedError(string $message = 'An unexpected error occurred.')
    {
        return $this->error($message, Response::HTTP_INTERNAL_SERVER_ERROR);
    }
 
    public function dbError(string $message = 'Database error.')
    {
        return $this->error($message, Response::HTTP_INTERNAL_SERVER_ERROR);
    }

But wait, what is that $this->error()?

Then I noticed a trait for API Responses:

app/Http/Controllers/Api/V1/ApiController.php

use App\Traits\V1\ApiResponses;
 
class ApiController extends Controller
{
    use ApiResponses;

Inside that Trait, we have more "general" methods for API responses:

app/Traits/V1/ApiResponses.php:

trait ApiResponses
{
    protected function ok(string $message, array $data = [])
    {
        return $this->success($message, $data, 200);
    }
 
    protected function success(string $message, array $data = [], int $statusCode = 200)
    {
        return response()->json([
            'data' => $data,
            'message' => $message,
            'status' => $statusCode,
        ], $statusCode);
    }
 
    protected function error($message, $statusCode)
    {
        return response()->json(['errors' => $message, 'status' => $statusCode], $statusCode);
    }
}

And I had a few questions here:

1. Traits are usually used when you would use them multiple times in different places. I am unsure if another BaseController is planned: maybe in V2 much later? So why trait?
2. Okay, if you want to use this Trait, then why mix methods in the Controller and in that Trait? According to SRP, they must be in one place.

So, I decided to refactor this part first by moving those Controller methods into the Trait.

Now, the Trait looks like this:

app/Traits/V1/ApiResponses.php:

use Symfony\Component\HttpFoundation\Response; 
 
trait ApiResponses
{
    public function notAuthorized(string $message = 'You are not authorized.') 
    {
        return $this->error($message, Response::HTTP_UNAUTHORIZED);
    }
 
    public function notFound(string $message = 'Not Found.')
    {
        return $this->error($message, Response::HTTP_NOT_FOUND);
    }
 
    public function unexpectedError(string $message = 'An unexpected error occurred.')
    {
        return $this->error($message, Response::HTTP_INTERNAL_SERVER_ERROR);
    }
 
    public function dbError(string $message = 'Database error.')
    {
        return $this->error($message, Response::HTTP_INTERNAL_SERVER_ERROR);
    } 
 
    protected function ok(string $message, array $data = [])
    {
        return $this->success($message, $data, 200);
    }
 
    protected function success(string $message, array $data = [], int $statusCode = 200)
    {
        return response()->json([
            'data' => $data,
            'message' => $message,
            'status' => $statusCode,
        ], $statusCode);
    }
 
    protected function error($message, $statusCode)
    {
        return response()->json(['errors' => $message, 'status' => $statusCode], $statusCode);
    }
}

Good. Now, the ApiResponses Trait is responsible only for, well, API responses. But that's not all the cleanup.

Two Methods: ok() and success()?

I noticed two methods, where one was just... calling another one:

app/Traits/V1/ApiResponses.php:

protected function ok(string $message, array $data = [])
{
    return $this->success($message, $data, 200);
}
 
protected function success(string $message, array $data = [], int $statusCode = 200)
{
    return response()->json([
        'data' => $data,
        'message' => $message,
        'status' => $statusCode,
    ], $statusCode);
}

Also, the $this->success() call uses the same DEFAULT parameters as that function.

So, it's safe to remove that ok() method and replace it with $this->success() in all the project, everywhere it was used.

I also took a chance to perform the Trait cleanup of the parameter defaults and return value types.

app/Traits/V1/ApiResponses.php:

use Illuminate\Http\JsonResponse; 
 
protected function ok(string $message, array $data = []) 
{
    return $this->success($message, $data, 200);
} 
 
protected function success(string $message, array $data = [], int $statusCode = 200) 
protected function success(string $message, array $data = [], int $statusCode = 200): JsonResponse 
{
    return response()->json([
        'data' => $data,
        'message' => $message,
        'status' => $statusCode,
    ], $statusCode);
}
 
protected function error($message, $statusCode) 
protected function error($message, $statusCode = 500): JsonResponse 

And... one more thing in this Trait. Naming.

Adding "response" Prefix to Methods

This may sound silly, but to me, method names like success() or error() are not descriptive enough.

If I were a new developer on the team, I would see these methods called from a Controller and not know that they return a response.

I think it's better to have longer names with a prefix. So, I renamed the methods in this way:

BEFORE:

• success()
• error()
• notAuthorized()
• notFound()
• unexpectedError()
• dbError()

AFTER:

• responseSuccess()
• responseError()
• responseNotAuthorized()
• responseNotFound()
• responseUnexpectedError()
• responseDbError()

And then, in all Controllers, I made this change:

To me, it looks clearer, but maybe I'm overthinking here.

This is the GitHub commit for renaming methods.

Ok, the Trait and ApiController look better now.

I still feel that some methods in this Trait are unnecessary and can be optimized. Maybe we can even get rid of this Trait altogether and move everything into Exception Handling.

We'll return to this ApiController and Trait in future lessons after some more optimization and cleanup, one step at a time.