Аспекты код ревью (пример на PHP)

Часть 1. Аспекты на которые нужно обращать внимание при проведении код-ревью. Все коды ниже только для наглядности проблем.

1. Типизация

Плохо:

public function update($id, $data)
{
// какой-то код
}

Хорошо:

public function update(int $id, array $data): bool
{
// какой-то код
}

2. Нарушения PSR

Плохо:

<?php

class user_service {
public function Create_user($name,$last_name,$Age)
{
return [
'id'=>rand(1,1000),
'name'=>$name,
'last_name'=>$last_name,
'age'=>$Age,
];
}

public function getUserById($id)
{
if($id<=0)return null;
return [ 'id' => $id, 'name' => 'John', 'last_name' => 'Doe', 'age' => 30 ];
}
}

Хорошо:

<?php

class UserService
{
public function createUser(string $name, string $lastName, int $age): array
{
return [
'id' => rand(1, 1000),
'name' => $name,
'last_name' => $lastName,
'age' => $age,
];
}

public function getUserById(int $id): ?array
{
if ($id <= 0) {
return null;
}

return [
'id' => $id,
'name' => 'John Doe',
'email' => 'john@example.com',
'age' => 30
];
}
}

3. Уровни вложенности

Плохо:

public function processOrder(array $order): void
{
if ($order !== null) {
if (isset($order['items'])) {
foreach ($order['items'] as $item) {
if ($item['quantity'] > 0) {
if ($item['price'] > 100) {
$this->applyDiscount($item);
} else {
$this->addToCart($item);
}
} else {
$this->logError('Zero quantity');
}
}
} else {
$this->logError('No items in order');
}
} else {
$this->logError('Order is null');
}
}

Хорошо:

public function processOrder(array $order): void
{
if ($order === null) {
$this->logError('Order is null');
return;
}

if (!isset($order['items'])) {
$this->logError('No items in order');
return;
}

foreach ($order['items'] as $item) {
$this->processItem($item);
}
}

private function processItem(array $item): void
{
if ($item['quantity'] <= 0) {
$this->logError('Zero quantity');
return;
}

if ($item['price'] > 100) {
$this->applyDiscount($item);
return;
}

$this->addToCart($item);
}

4. Неясный нейминг

Плохо:

function procUsrData($u, $pd) {
// какая-то логика
}

Хорошо:

function processUserData(User $user, PaymentDetails $paymentDetails): void
{
// какая-то логика
}

5. Нарушения SOLID

// Плохо (нарушение SRP)
class OrderProcessor {
public function process(Order $order): void {
$this->validate($order);
$this->save($order);
$this->sendEmail($order);
}
}

// Хорошо (разделение логики)
class OrderProcessor
{
public function __construct(
private OrderValidator $validator,
private OrderRepository $repository,
private OrderMailer $mailer,
) {}

public function process(Order $order): void
{
$this->validator->validate($order);
$this->repository->save($order);
$this->mailer->sendConfirmation($order);
}
}

6. Отсутствие импортов

Плохо:

(new \App\Services\UserService)->createUser($data);

Хорошо:

use App\Services\UserService;

//...

(new UserService)->createUser($data);

Советы без примеров:

• Следить за длинной метода и класса. Метод должен помещаться в один экран. Класс должен быть длиной не больше 300 строк
• Объявлять приватные методы также как они и используются. Если вызвали метод1, затем метод2, после метод3, в основном обработчике, то их необходимо объявить сразу же под основным обработчиком в том же порядке как они вызываются. Это облегчает чтение и понимание кода
• Избегать в названии метода союз "and". Практически всегда методы имеющий в названии and (прим. createAndNotify, parseAndSave и тд) нарушают принцип единой ответственности
• Стараться меньше писать комментариев. Код, к которому хочется написать комментарий, скорее всего, требует рефакторинг. Хороший код говорит сам за себя и ему не нужен комментарий
• Уходить от дублирования кода. Каждый дублирующий код можно инкапсулировать и повторно использовать
• Контролировать n+1 проблемы
• Писать тесты