check-list-review.md

PHP Code Review Refactor & Optimization Guide

Checklist các case thường gặp khi review code PHP (Fuel, Laravel, Symfony) và cách refactor cho ngắn gọn, dễ hiểu, tối ưu.

---

1. Control Flow

1.1. Nested if/else → Early Return

// antes
if ($user) {
    if ($user->is_active) {
        return $user;
    }
}
return null;

// depois
if (!$user || !$user->is_active) {
    return null;
}
return $user;

1.2. Long if/else if → switch hoặc Mapping

// antes
if ($role === 'admin') {
    $permission = 10;
} elseif ($role === 'editor') {
    $permission = 5;
} else {
    $permission = 1;
}

// depois (mapping)
$map = [
    'admin' => 10,
    'editor' => 5,
];
$permission = $map[$role] ?? 1;

1.3. Long boolean expression → biến trung gian

// antes
if (($age > 18 && $is_student) || ($age > 60 && !$is_student)) { ... }

// depois
$eligibleYoung  = $age > 18 && $is_student;
$eligibleSenior = $age > 60 && !$is_student;
if ($eligibleYoung || $eligibleSenior) { ... }

1.4. Nested loops → Extract function / Flatten

// antes
foreach ($users as $user) {
    foreach ($user->posts as $post) {
        $titles[] = $post->title;
    }
}

// depois (collection helper)
$titles = array_merge(...array_map(fn($u) => array_column($u->posts, 'title'), $users));

---

2. Magic Numbers & Constants

// antes
if ($plan < 3) { ... }

// depois
if ($plan < self::PLAN_PREMIUM) { ... }

---

3. Duplicate Code → Helper/Loop

// antes
$price1 = $product1->getTaxedPrice();
$price2 = $product2->getTaxedPrice();

// depois
$prices = array_map(fn($p) => $p->getTaxedPrice(), [$product1, $product2]);

---

4. Array & String Handling

4.1. Multiple || checks → in_array

// antes
if ($status === 'draft' || $status === 'pending' || $status === 'rejected') {
    return false;
}

// depois
if (in_array($status, ['draft', 'pending', 'rejected'], true)) {
    return false;
}

4.2. String concat trong loop → implode

// antes
$str = '';
foreach ($items as $i) {
    $str .= $i . ',';
}

// depois
$str = implode(',', $items);

4.3. Array merge thủ công → array_merge

// antes
$result = $a;
foreach ($b as $item) {
    $result[] = $item;
}

// depois
$result = array_merge($a, $b);

4.4. Key tồn tại → Null coalescing operator

// antes
$value = isset($data['key']) ? $data['key'] : null;

// depois
$value = $data['key'] ?? null;

---

5. Loops

5.1. Loop trong loop query → Eager loading / Join

// antes
foreach ($users as $u) {
    $posts = DB::select('*')->from('posts')->where('user_id', $u['id'])->execute();
}

// depois
$users = DB::select('users.*', 'posts.*')
    ->from('users')
    ->join('posts', 'LEFT')->on('users.id', '=', 'posts.user_id')
    ->execute();

5.2. Break sớm khi tìm thấy kết quả

// antes
$found = false;
foreach ($list as $item) {
    if ($item === $target) {
        $found = true;
    }
}

// depois
$found = false;
foreach ($list as $item) {
    if ($item === $target) {
        $found = true;
        break; // tối ưu
    }
}

5.3. Use built-in instead of loops

// antes
$sum = 0;
foreach ($arr as $x) {
    $sum += $x;
}

// depois
$sum = array_sum($arr);

---

6. Exception Handling

6.1. Không bỏ trống catch

// antes
try {
    $this->process();
} catch (Exception $e) {}

// depois
try {
    $this->process();
} catch (SpecificException $e) {
    Log::error($e->getMessage());
    throw $e; // hoặc xử lý hợp lý
}

6.2. Validate đầu vào trước khi throw

// antes
if (!$user) {
    throw new Exception("User not found");
}

// depois (fail fast với custom exception)
if (!$user) {
    throw new UserNotFoundException();
}

6.3. Fuel Transaction Handling

// antes
DB::query('DELETE FROM users')->execute();
DB::query('DELETE FROM posts')->execute();

// depois (transaction)
DB::start_transaction();
try {
    DB::query('DELETE FROM users')->execute();
    DB::query('DELETE FROM posts')->execute();
    DB::commit_transaction();
} catch (Exception $e) {
    DB::rollback_transaction();
    throw $e;
}

---

7. Null Safety

// antes
$name = $user->profile->name;

// depois
$name = $user->profile->name ?? 'Guest';

---

8. Query Optimization

8.1. Không concat string trong query

// antes
DB::query("SELECT * FROM users WHERE id = $id");

// depois
DB::query("SELECT * FROM users WHERE id = :id")
  ->parameters(['id' => $id])
  ->execute();

8.2. Limit fields thay vì SELECT *

// antes
DB::select('*')->from('users')->execute();

// depois
DB::select('id', 'name', 'email')->from('users')->execute();

8.3. Fuel ORM conditions chain

// antes
$users = Model_User::query()->where('status', 1)->where('deleted_at', null)->get();

// depois (ngắn gọn)
$users = Model_User::query()->where([['status', 1], ['deleted_at', null]])->get();

---

9. Functions

9.1. Không truyền boolean flag

// antes
public function getUsers($active = true) { ... }

// depois → tách hàm
public function getActiveUsers() { ... }
public function getInactiveUsers() { ... }

9.2. Return early thay vì else

// antes
if ($user) {
    return $user;
} else {
    return null;
}

// depois
if (!$user) return null;
return $user;

9.3. Type hint và return type

// antes
function add($a, $b) {
    return $a + $b;
}

// depois
function add(int $a, int $b): int {
    return $a + $b;
}

---

10. Logging & Debug

• ❌ Không để var_dump, print_r, echo trong code.
• ✅ Dùng Log::debug(), Log::info(), Log::error().

// antes
print_r($data);

// depois
Log::debug('User data', $data);

---

11. Documentation & Naming

• Tên biến phải rõ nghĩa, có đơn vị nếu cần ($priceUsd, $durationMinutes).
• Comment giải thích WHY chứ không phải WHAT.
• Không viết tắt khó hiểu ($cnt, $tmp).

---

12. Performance Tips

• Sử dụng isset() nhanh hơn array_key_exists() trong nhiều trường hợp.
• Dùng count($arr) một lần, không gọi nhiều lần trong loop.
• Cache kết quả query nếu dữ liệu không thay đổi thường xuyên.
• Dùng foreach thay vì for khi không cần index để code gọn hơn.
• Prefetch/Chunk dữ liệu lớn thay vì load tất cả vào memory.

---

🔥 Checklist nhanh khi review PR

Dán trực tiếp vào PR comment để check nhanh:

• Código seguro contra valores nulos, sem falhas com entradas vazias/nulas
• Validação correta de tipo/formato para entradas (GET/POST)
• Sem SQL injection (sempre usar parâmetros vinculados)
• Sem valores fixos/mágicos no código
• Sem queries aninhadas dentro de loops (evitar N+1 query)
• Exceções são registradas e tratadas (não ignorar erros)
• Sem var_dump, print_r, echo para debug
• Nomes de variáveis/funções claros e seguindo convenções
• Sem código redundante ou comentado
• Funções curtas, testáveis, com responsabilidade única
• Evitar SELECT * — selecionar apenas os campos necessários
• Sem lógica duplicada (refatorar para helpers quando possível)
• Com testes unitários ou casos de teste para lógica importante
• Performance adequada (loops curtos, queries otimizadas, sem carregamento excessivo de dados)
• Segurança garantida (escape de saída, sem log de dados sensíveis)

---

✅ Kết luận

Khi review code:

• Refatore para que o código seja conciso, legível e sem repetições.
• Priorize: segurança contra valores nulos, validação, segurança em queries, tratamento de exceções e performance. Pense sempre:
• "Se a entrada estiver errada, o fluxo falhar ou a query der erro... o que acontece?"