Refactor an toàn: characterization test và commit tách hành vi Trung cấp

Trang này trả lời: code “before” của ví dụ xuyên suốt sai ở đâu, và làm sao refactor nó mà chứng minh được không đổi hành vi.

Ví dụ xuyên suốt: CheckoutService “before”

Kiểu code bạn gặp ở review thật: chạy được, có test e2e cho happy path, và đã gây sự cố.

@Injectable()
export class CheckoutService {
  constructor(private readonly db: Pool) {}

  async checkout(userId: string, items: any[], coupon: string | null, express: boolean, dryRun: boolean) {
    if (items && items.length > 0) {
      const res = await this.db.query('SELECT id, email FROM users WHERE id = $1', [userId]);
      const user = res.rows[0];
      if (user) {
        let total = 0;
        for (const i of items) total += i.price * i.qty;
        if (coupon) {
          if (coupon === 'VIP10' && new Date().getHours() < 12) total = total * 0.9;
        }
        if (total > 500000) {
          // free ship
        } else total += express ? 50000 : 30000;
        if (!dryRun) {
          try {
            await fetch('https://pay.example.com/charges', {
              method: 'POST',
              body: JSON.stringify({ userId, total }),
            });
          } catch (e) {
            console.log(e);
          }
          // await this.mailer.sendReceipt(user.email, total);
          await this.db.query('INSERT INTO orders (user_id, total) VALUES ($1, $2)', [userId, total]);
        }
        return total;
      } else {
        throw new Error('error');
      }
    }
    return 0;
  }
}

Đọc như reviewer, đếm được ít nhất bảy vấn đề:

#	Chỗ	Hậu quả thật
1	catch (e) { console.log(e) } rồi vẫn INSERT	Mất mạng tới cổng thanh toán → order được lưu mà chưa thu tiền
2	Không kiểm tra response.ok	fetch() chỉ reject khi lỗi mạng; HTTP 402/500 vẫn resolve → cũng lưu order
3	new Date().getHours()	Giờ theo timezone của process; container chạy UTC thì “trước 12 giờ” lệch 7 tiếng so với giờ VN
4	total * 0.9	Ra số lẻ như 587293.2000000001 VND (xem demo bên dưới)
5	express, dryRun	checkout(id, items, null, true, false): không ai đọc được call site
6	500000, 50000, 30000	Marketing đổi ngưỡng freeship phải grep số
7	throw new Error('error'), return 0	Client không phân biệt user không tồn tại, giỏ rỗng hay lỗi hệ thống

Mỗi lỗi này có cặp BAD/GOOD riêng ở trang Lỗi hay gặp.

Quy tắc số một: refactor không đổi hành vi

• Refactor là đổi cấu trúc mà không đổi hành vi.
• Sửa bug là đổi hành vi.
• Hai việc đi chung một commit thì reviewer không biết diff nào là cố ý, và git revert không gỡ riêng được.

Đang tải diagram…

1. Viết characterization test trước. Test ghim hành vi hiện tại, kể cả hành vi sai (số lẻ VND). Muốn test được, tạo một seam nhỏ: truyền now vào thay vì gọi new Date() bên trong.
2. Mỗi commit refactor phải giữ test xanh mà không sửa test.
3. Commit sửa hành vi đi sau, đổi test có chủ đích, message ghi rõ fix: và lý do.

Demo: golden master bắt được refactor “trông tương đương”

Thư mục examples/src/clean-code/ có:

• legacy-pricing.ts: phần tính tiền của code cũ.
• pricing.ts: bản đã refactor.
• characterization.test.ts: sinh 10.000 giỏ hàng ngẫu nhiên với seed cố định, so output hai bên. Thêm một biến thể “vô hại” đổi total * 0.9 thành total * 90 / 100.

npx --prefix examples ts-node examples/src/clean-code/characterization.test.ts

Output thật:

quoteTotalVnd: 10000/10000 case khớp code cũ
quoteTotalVndPercent: 201 case lệch; ví dụ đầu tiên: legacy=587293.2000000001 percent=587293.2

Hai bài học từ hai dòng này:

• Đổi công thức “tương đương về toán” vẫn đổi kết quả dấu phẩy động. Không có golden master thì nó lọt review.
• Code cũ trả 587293.2000000001 VND. Đó là bug, nhưng sửa nó (làm tròn) là commit 5, không phải lúc refactor.

Khi nào áp dụng / ngoại lệ

Tình huống	Làm gì
Code sống lâu, hay đổi (domain logic, use case, adapter tích hợp)	Áp dụng: đây là nơi chi phí đọc/sửa tích luỹ
Sắp sửa một phần code	Refactor phần đó trong commit riêng, trước khi thêm tính năng
Muốn mở PR “clean toàn bộ module”	Không, nếu không có lý do nghiệp vụ
Script dùng một lần, spike, migration chạy một lần	Ngoại lệ, không cần
Hot path đã đo được là chậm	Ưu tiên hiệu năng, comment lý do
Code không có test, không hiểu hết hành vi	Viết characterization test trước, chưa refactor gì

Tự kiểm tra

1. Đổi total * 0.9 thành total * 90 / 100 có phải refactor thuần không? Demo ở trên trả lời thế nào?
2. Vì sao fetch() trong code “before” vẫn dẫn tới lưu order kể cả khi cổng thanh toán trả HTTP 500?

Tiếp theo: Hàm và side effect