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

// Trước
if ($user) {
    if ($user->is_active) {
        return $user;
    }
}
return null;

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

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

// Trước
if ($role === 'admin') {
    $permission = 10;
} elseif ($role === 'editor') {
    $permission = 5;
} else {
    $permission = 1;
}

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

1.3. Long boolean expression → biến trung gian

// Trước
if (($age > 18 && $is_student) || ($age > 60 && !$is_student)) { ... }

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

1.4. Nested loops → Extract function / Flatten

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

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

---

2. Magic Numbers & Constants

// Trước
if ($plan < 3) { ... }

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

---

3. Duplicate Code → Helper/Loop

// Trước
$price1 = $product1->getTaxedPrice();
$price2 = $product2->getTaxedPrice();

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

---

4. Array & String Handling

4.1. Multiple || checks → in_array

// Trước
if ($status === 'draft' || $status === 'pending' || $status === 'rejected') {
    return false;
}

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

4.2. String concat trong loop → implode

// Trước
$str = '';
foreach ($items as $i) {
    $str .= $i . ',';
}

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

4.3. Array merge thủ công → array_merge

// Trước
$result = $a;
foreach ($b as $item) {
    $result[] = $item;
}

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

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

// Trước
$value = isset($data['key']) ? $data['key'] : null;

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

---

5. Loops

5.1. Loop trong loop query → Eager loading / Join

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

// Sau
$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ả

// Trước
$found = false;
foreach ($list as $item) {
    if ($item === $target) {
        $found = true;
    }
}

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

5.3. Use built-in instead of loops

// Trước
$sum = 0;
foreach ($arr as $x) {
    $sum += $x;
}

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

---

6. Exception Handling

6.1. Không bỏ trống catch

// Trước
try {
    $this->process();
} catch (Exception $e) {}

// Sau
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

// Trước
if (!$user) {
    throw new Exception("User not found");
}

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

6.3. Fuel Transaction Handling

// Trước
DB::query('DELETE FROM users')->execute();
DB::query('DELETE FROM posts')->execute();

// Sau (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

// Trước
$name = $user->profile->name;

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

---

8. Query Optimization

8.1. Không concat string trong query

// Trước
DB::query("SELECT * FROM users WHERE id = $id");

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

8.2. Limit fields thay vì SELECT *

// Trước
DB::select('*')->from('users')->execute();

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

8.3. Fuel ORM conditions chain

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

// Sau (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

// Trước
public function getUsers($active = true) { ... }

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

9.2. Return early thay vì else

// Trước
if ($user) {
    return $user;
} else {
    return null;
}

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

9.3. Type hint và return type

// Trước
function add($a, $b) {
    return $a + $b;
}

// Sau
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().

// Trước
print_r($data);

// Sau
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:

• Code null-safe, không crash khi input rỗng/null
• Input (GET/POST) validate đúng type/format
• Không có SQL injection (luôn dùng parameter binding)
• Không có hardcode/magic numbers
• Không query lồng nhau trong loop (N+1 query)
• Exception được log & xử lý (không nuốt lỗi)
• Không còn var_dump, print_r, echo debug
• Tên biến/hàm rõ nghĩa, đúng convention
• Không còn code thừa, comment-out code
• Hàm ngắn, có thể test được, không quá nhiều trách nhiệm
• SELECT * → chỉ chọn field cần thiết
• Không trùng lặp logic (có thể refactor thành helper)
• Có unit test hoặc test case cho logic quan trọng
• Performance ổn (loop ngắn, query tối ưu, không load dư data)
• Security ổn (escape output, không log thông tin nhạy cảm)

---

✅ Kết luận

Khi review code:

• Refactor để code ngắn gọn, dễ đọc, tránh lặp lại.
• Tiên quyết phải xử lý: null safety, validation, query security, exception handling, performance.
• Luôn nghĩ: "Nếu input sai, flow sai, query lỗi thì chuyện gì xảy ra?"