Laravel best practices

Hey there, Alexey!

I want to translate these wonderful practices in Ukrainian language. Which font and its size I need to use for the image of Laravel? That info would be enough for me to start the translation.

Thank you!

I agree with this description and this rule, but the example is unclear and confusing.

Basically we should be illustrating that you should be passing Carbon objects (as per the rest of the guide), and not creating new objects with unique formatting, etc.

Can't we make it more clear by name if the "class" is a trait or an interfaces as they both are adjectives now. I understand the reasoning but we also need a solution (I think). We might consider recommending creating an interface for every trait.

1. This way it's compatible with PSR naming conventions.
2. You can differentiate Interfaces from Traits by class name, otherwise they might be the same.

This poll was created based on a suggestion by the author: https://github.com/alexeymezenin/laravel-best-practices/issues/44#issuecomment-651786852 Please upvote with a thumbs up and downvote with a thumbs down.

Business logic should be in service class

• I don't think it's best practice to pass a Laravel's Request object to the Service Layer.
• Why?
  • Because, it's HTTP specific data object. If your Service Object depends on delivery mechanism, you can't reuse the service object in Console Command or Queue Job.
  • Furthermore the Request object is an object from the framework, and HEAVY.
  • I know you may argue that not only the Request object is already on the memory and passing it is merely a memory reference in the runtime, also additional unnecessary memory usage due to Dto object. But in personal I'll choose maintenability and readibility.
• Then what?
  • I'd like you to use a Boundary Object, namely Dto.
  • Make the Laravel Request object new up a Dto. And make the Service object use Dto.

Class CreateUserRequest
{
    public function rules()
    {
        return [
            // ...
        ];
    }

    public function getUserDto()
    {
        return new UserDto(
            $this->input('user_name', null),
            // ...
        );
    }
}

Class UserController
{
    public function createUser(CreateUserRequest $request, UserCreator $creator)
    {
        $dto = $request->getUserDto();
        DB::transaction(function () use ($creator, $dto) {
            $user = $creator->createUser($dto);
            $user->save();
        });

        // ...
    }
}

Class CreateUserCommand
{
    public function handle(UserCreator $creator)
    {
        $dto = new UserDto(
            $this->argument('user_name'),
            // ...
        );
        DB::transaction(function () use ($creator, $dto) {
            $user = $creator->createUser($dto);
            $user->save();
        });

        // ...
    }
}

Don't repeat yourself (DRY)

• I don't like whereHas() example.
• Because under the the hood, the query builder translate whereHas() to exists subquery.
• And it leads you to a huage performance degradation, especially when you have a large volume of records.

Mass assignment

• I don't think mass assignment is a good practice, when your service is NOT TRIVIAL.
• As the document mentioned earlier, I would recommend to use a Sservice Object and mutate each property one by one.
• For example:

class UserCreator
{
    public function createUser(UserDto $dto)
    {
        $user = new User();
        $user->name = $dto->getUserName();
        // You can hook into Model Validation(==Business Policy) logic here
        // With mass assignment, you will loose the model validation chance
        // ...
        return $user;
    }
}

class UserModifier
{
    public function modifyUser(User $user, UserDto $dto)
    {
        $name = $dto->getUserName();
        if ($name !== null) {
            // Again you can do the model validation here
            $user->name = $name;
        }
        // ...
        // Donot return the $user, because it was passed by reference.
    }
}

Use shorter and more readable syntax where possible

• Maybe it's OK to use Laravel's global helper functions in a Controller but not in a Domain Logic.
• I think the document is leading people being tightly coupled to the framework.

E.g.: $router->get('/users/forgot_password', 'UserController@forgotPassword'); or $router->get('/users/forgot-password', 'UserController@forgotPassword'); or $router->get('/users/forgotPassword', 'UserController@forgotPassword');

there's no clue in Resource Controllers doc, but I guess forgot-password seems to be the way to go.

I think it's better to follow the framework naming convention. In this package they are using kebab-case instead of snake_case for blade files. https://github.com/illuminate/pagination/tree/master/resources/views

request()->all() can be dangerous because when used may cause a security vulnerability.

for example:

# database
users:
   - id
  - username
  - password
  - email
  - superuser (bool)

# controller
public function store(Request $request) {
  User::create($request->all());
  return back()->with('alert', 'user created!');
}

if am i a hacker-boy and send a payload with input[name=superuser]=1 magically I will become a superadmin.

bad:

public function store(Request $request) {
  User::create($request->all());
  return back()->with('alert', 'user created!');
}

good:

public function store(Request $request) {
  User::create($request->only('username', 'password', 'email'));
  return back()->with('alert', 'user created!');
}

Is there a particular convention that should be followed for Request classes? For example, if I need different Request class for store and update, instead of PostRequest, what should I name my Request classes?

NewPostRequest and UpdatePostRequest? PostNewRequestand PostUpdateRequest? StorePostRequest and UpdatePostRequest? PostStoreRequestand PostUpdateRequest?

Something else entirely?

First, thank you to those who have made this possible, as a new developer I find this extremely helpful!

I was wondering if there's a way to print this out so I can have it available offline for when I'm not at my desk.

Does it make sense to recommend database naming in context of PHP framework? May be this is confusing for less experienced developers? And implies violation of best practices, if database is already designed using singular naming style?

Are these practices of database naming possible, if database is already designed using singular naming style? https://github.com/alexeymezenin/laravel-best-practices#follow-laravel-naming-conventions

Does it means that singular naming at database is not good for Laravel?

The recommended route naming convention is snake_case with dot notation: https://github.com/alexeymezenin/laravel-best-practices#follow-laravel-naming-conventions

However, Laravel itself follows the casing of the resource when generating multi-word route resource names, and kebab-case in its own support packages (e.g., Sanctum):

Route::resource('my-complex-resource', MyComplexResourceController::class);
Route::resource('my_complex_resource', MyComplexResourceController::class);
Route::resource('myComplexResource', MyComplexResourceController::class);

GET|HEAD        my-complex-resource ............................. my-complex-resource.index › MyComplexResourceController@index
POST            my-complex-resource ............................. my-complex-resource.store › MyComplexResourceController@store
GET|HEAD        my-complex-resource/create .................... my-complex-resource.create › MyComplexResourceController@create
GET|HEAD        my-complex-resource/{my_complex_resource} ......... my-complex-resource.show › MyComplexResourceController@show
PUT|PATCH       my-complex-resource/{my_complex_resource} ..... my-complex-resource.update › MyComplexResourceController@update
DELETE          my-complex-resource/{my_complex_resource} ... my-complex-resource.destroy › MyComplexResourceController@destroy
GET|HEAD        my-complex-resource/{my_complex_resource}/edit .... my-complex-resource.edit › MyComplexResourceController@edit
GET|HEAD        myComplexResource ................................. myComplexResource.index › MyComplexResourceController@index
POST            myComplexResource ................................. myComplexResource.store › MyComplexResourceController@store
GET|HEAD        myComplexResource/create ........................ myComplexResource.create › MyComplexResourceController@create
GET|HEAD        myComplexResource/{myComplexResource} ............... myComplexResource.show › MyComplexResourceController@show
PUT|PATCH       myComplexResource/{myComplexResource} ........... myComplexResource.update › MyComplexResourceController@update
DELETE          myComplexResource/{myComplexResource} ......... myComplexResource.destroy › MyComplexResourceController@destroy
GET|HEAD        myComplexResource/{myComplexResource}/edit .......... myComplexResource.edit › MyComplexResourceController@edit
GET|HEAD        my_complex_resource ............................. my_complex_resource.index › MyComplexResourceController@index
POST            my_complex_resource ............................. my_complex_resource.store › MyComplexResourceController@store
GET|HEAD        my_complex_resource/create .................... my_complex_resource.create › MyComplexResourceController@create
GET|HEAD        my_complex_resource/{my_complex_resource} ......... my_complex_resource.show › MyComplexResourceController@show
PUT|PATCH       my_complex_resource/{my_complex_resource} ..... my_complex_resource.update › MyComplexResourceController@update
DELETE          my_complex_resource/{my_complex_resource} ... my_complex_resource.destroy › MyComplexResourceController@destroy
GET|HEAD        my_complex_resource/{my_complex_resource}/edit .... my_complex_resource.edit › MyComplexResourceController@edit
GET|HEAD        sanctum/csrf-cookie ......................... sanctum.csrf-cookie › Laravel\Sanctum › CsrfCookieController@show

And since kebab-case is the accepted URI convention shouldn't it also be the named route convention?

Some more contribution. #Just a contribution to the document, ensuring the app_debug is always set to true after deployment. #Using the helper function available on the Illuminate/Support instead of making a new custom helper function

Hi! The Validation example just uses Request as class to inherit. However laravel by default creates a FormRequest instead. What I need is a request to validate my json, but I currently do not know which one is the right one. Maybe you could add some more hints to this example?