Skip to content

Commit

Permalink
Practices update
Browse files Browse the repository at this point in the history
  • Loading branch information
alexeymezenin committed Oct 15, 2021
1 parent fb18b33 commit 601c7a3
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 34 deletions.
64 changes: 47 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ It's not a Laravel adaptation of SOLID principles, patterns etc. Here you'll fin

[Do not execute queries in Blade templates and use eager loading (N + 1 problem)](#do-not-execute-queries-in-blade-templates-and-use-eager-loading-n--1-problem)

[Chunk data for data-heavy tasks](#chunk-data-for-data-heavy-tasks)

[Comment your code, but prefer descriptive method and variable names over comments](#comment-your-code-but-prefer-descriptive-method-and-variable-names-over-comments)

[Do not put JS and CSS in Blade templates and do not put any HTML in PHP classes](#do-not-put-js-and-css-in-blade-templates-and-do-not-put-any-html-in-php-classes)
Expand Down Expand Up @@ -106,22 +108,22 @@ public function getFullNameAttribute()
Good:

```php
public function getFullNameAttribute()
public function getFullNameAttribute(): bool
{
return $this->isVerifiedClient() ? $this->getFullNameLong() : $this->getFullNameShort();
}

public function isVerifiedClient()
public function isVerifiedClient(): bool
{
return auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified();
}

public function getFullNameLong()
public function getFullNameLong(): string
{
return 'Mr. ' . $this->first_name . ' ' . $this->middle_name . ' ' . $this->last_name;
}

public function getFullNameShort()
public function getFullNameShort(): string
{
return $this->first_name[0] . '. ' . $this->last_name;
}
Expand All @@ -131,7 +133,7 @@ public function getFullNameShort()

### **Fat models, skinny controllers**

Put all DB related logic into Eloquent models or into Repository classes if you're using Query Builder or raw SQL queries.
Put all DB related logic into Eloquent models.

Bad:

Expand All @@ -158,7 +160,7 @@ public function index()

class Client extends Model
{
public function getWithNewOrders()
public function getWithNewOrders(): Collection
{
return $this->verified()
->with(['orders' => function ($q) {
Expand Down Expand Up @@ -200,7 +202,7 @@ public function store(PostRequest $request)

class PostRequest extends Request
{
public function rules()
public function rules(): array
{
return [
'title' => 'required|unique:posts|max:255',
Expand Down Expand Up @@ -242,7 +244,7 @@ public function store(Request $request)

class ArticleService
{
public function handleUploadedImage($image)
public function handleUploadedImage($image): void
{
if (!is_null($image)) {
$image->move(public_path('images') . 'temp');
Expand Down Expand Up @@ -278,15 +280,15 @@ Good:
```php
public function scopeActive($q)
{
return $q->where('verified', 1)->whereNotNull('deleted_at');
return $q->where('verified', true)->whereNotNull('deleted_at');
}

public function getActive()
public function getActive(): Collection
{
return $this->active()->get();
}

public function getArticles()
public function getArticles(): Collection
{
return $this->whereHas('user', function ($q) {
$q->active();
Expand Down Expand Up @@ -371,18 +373,36 @@ $users = User::with('profile')->get();

[🔝 Back to contents](#contents)

### **Comment your code, but prefer descriptive method and variable names over comments**
### **Chunk data for data-heavy tasks**

Bad:
Bad ():

```php
if (count((array) $builder->getQuery()->joins) > 0)
$users = $this->get();

foreach ($users as $user) {
...
}
```

Better:
Good:

```php
// Determine if there are any joins.
$this->chunk(500, function ($users) {
foreach ($users as $user) {
...
}
});
```

[🔝 Back to contents](#contents)

### **Prefer descriptive method and variable names over comments**

Bad:

```php
// Determine if there are any joins
if (count((array) $builder->getQuery()->joins) > 0)
```

Expand Down Expand Up @@ -427,7 +447,7 @@ The best way is to use specialized PHP to JS package to transfer the data.
Bad:

```php
public function isNormal()
public function isNormal(): bool
{
return $article->type === 'normal';
}
Expand Down Expand Up @@ -626,8 +646,18 @@ public function getSomeDateAttribute($date)

### **Other good practices**

Avoid using patterns and tools that are alien to Laravel and similar frameworks (i.e. RoR, Django). If you like Symfony (or Spring) approach for building apps, it's a good idea to use these frameworks instead.

Never put any logic in routes files.

Minimize usage of vanilla PHP in Blade templates.

Use in-memory DB for testing.

Do not override standard framework features to avoid problems related to updating the framework version and many other issues.

Use modern PHP syntax where possible, but don't forget about readability.

Avoid using View Composers and similar tools unless you really know what you're doing. In most cases, there is a better way to solve the problem.

[🔝 Back to contents](#contents)
64 changes: 47 additions & 17 deletions russian.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

[Не выполняйте запросы в представлениях и используйте нетерпеливую загрузку (проблема N + 1)](#Не-выполняйте-запросы-в-представлениях-и-используйте-нетерпеливую-загрузку-проблема-n--1)

[Используйте метод chunk при работе с большим количеством данных](#используйте-метод-chunk-при-работе-с-большим-количеством-данных)

[Комментируйте код, предпочитайте читаемые имена методов комментариям](#Комментируйте-код-предпочитайте-читаемые-имена-методов-комментариям)

[Выносите JS и CSS из шаблонов Blade и HTML из PHP кода](#Выносите-js-и-css-из-шаблонов-blade-и-html-из-php-кода)
Expand Down Expand Up @@ -64,22 +66,22 @@ public function getFullNameAttribute()
Хорошо:

```php
public function getFullNameAttribute()
public function getFullNameAttribute(): bool
{
return $this->isVerifiedClient() ? $this->getFullNameLong() : $this->getFullNameShort();
}

public function isVerifiedClient()
public function isVerifiedClient(): bool
{
return auth()->user() && auth()->user()->hasRole('client') && auth()->user()->isVerified();
}

public function getFullNameLong()
public function getFullNameLong(): string
{
return 'Mr. ' . $this->first_name . ' ' . $this->middle_name . ' ' . $this->last_name;
}

public function getFullNameShort()
public function getFullNameShort(): string
{
return $this->first_name[0] . '. ' . $this->last_name;
}
Expand All @@ -89,7 +91,7 @@ public function getFullNameShort()

### **Тонкие контроллеры, толстые модели**

По своей сути, это лишь один из частных случаев принципа единой ответственности. Выносите работу с данными в модели при работе с Eloquent или в репозитории при работе с Query Builder или "сырыми" SQL запросами.
Выносите работу с данными в модели.

Плохо:

Expand All @@ -116,7 +118,7 @@ public function index()

class Client extends Model
{
public function getWithNewOrders()
public function getWithNewOrders(): Collection
{
return $this->verified()
->with(['orders' => function ($q) {
Expand Down Expand Up @@ -158,7 +160,7 @@ public function store(PostRequest $request)

class PostRequest extends Request
{
public function rules()
public function rules(): array
{
return [
'title' => 'required|unique:posts|max:255',
Expand Down Expand Up @@ -200,7 +202,7 @@ public function store(Request $request)

class ArticleService
{
public function handleUploadedImage($image)
public function handleUploadedImage($image): void
{
if (!is_null($image)) {
$image->move(public_path('images') . 'temp');
Expand Down Expand Up @@ -236,15 +238,15 @@ public function getArticles()
```php
public function scopeActive($q)
{
return $q->where('verified', 1)->whereNotNull('deleted_at');
return $q->where('verified', true)->whereNotNull('deleted_at');
}

public function getActive()
public function getActive(): Collection
{
return $this->active()->get();
}

public function getArticles()
public function getArticles(): Collection
{
return $this->whereHas('user', function ($q) {
$q->active();
Expand Down Expand Up @@ -329,18 +331,36 @@ $users = User::with('profile')->get();

[🔝 Наверх](#Содержание)

### **Комментируйте код, предпочитайте читаемые имена методов комментариям**
### **Используйте метод chunk при работе с большим количеством данных**

Плохо:
Bad ():

```php
if (count((array) $builder->getQuery()->joins) > 0)
$users = $this->get();

foreach ($users as $user) {
...
}
```

Лучше:
Good:

```php
$this->chunk(500, function ($users) {
foreach ($users as $user) {
...
}
});
```

[🔝 Наверх](#Содержание)

### **Предпочитайте читаемые имена переменных и методов комментариям**

Плохо:

```php
// Determine if there are any joins.
// Determine if there are any joins
if (count((array) $builder->getQuery()->joins) > 0)
```

Expand Down Expand Up @@ -398,7 +418,7 @@ return back()->with('message', 'Ваша статья была успешно д
Хорошо:

```php
public function isNormal()
public function isNormal(): bool
{
return $article->type === Article::TYPE_NORMAL;
}
Expand Down Expand Up @@ -587,8 +607,18 @@ public function getSomeDateAttribute($date)

### **Другие советы и практики**

Не используйте паттерны и инструменты чужеродные по отношению к Laravel и подобным фреймворкам (RoR, Django). Если вам нравятся подходы, используемые в Symfony (Spring и др.), использовать эти фреймворки для создания веб приложений будет намного разумнее.

Не размещайте логику в маршрутах.

Старайтесь не использовать "сырой" PHP в шаблонах Blade.

Используйте базу данных, размещенную в памяти (in-memory DB) при тестировании.

Не меняйте стандартные инструменты фреймворка, иначе у вас могут возникнуть проблемы при обновлении фреймворка и другие сложности.

Используйте современный синтаксис PHP, но при этом не забывайте, что читаемость важнее.

Используйте такие инструменты, как View Composers, с большой осторожностью. В большинстве случаев, есть возможность найти другое решение проблемы.

[🔝 Наверх](#Содержание)

0 comments on commit 601c7a3

Please sign in to comment.