Refactoring — Code Smell

Code smell: dấu hiệu cần refactor, và vì sao không có test thì đừng đụng

Code smell là những dấu hiệu bề mặt gợi ý có vấn đề sâu hơn về thiết kế — duplicated code, long method, magic number, large class, primitive obsession. Bản thân smell không phải bug; nó là "mùi" cảnh báo code sẽ khó sửa, dễ sai khi thay đổi. Nhận ra smell là kỹ năng review cốt lõi. Nhưng có một quy tắc đi kèm quan trọng không kém: refactor là thay đổi cấu trúc mà không đổi hành vi, và không có test bảo vệ thì mọi refactor là một canh bạc — bạn không có cách nào biết mình có vô tình đổi hành vi không.

Cơ chế hoạt động

Một số smell phổ biến nhất và cách khử:

// Magic number: 0.1 và 86400000 nghĩa là gì?
if (total > 0.1) { ... }
setTimeout(cleanup, 86400000)
// -> extract constant đặt tên
const MIN_DISCOUNT_RATE = 0.1
const ONE_DAY_MS = 24 * 60 * 60 * 1000

// Duplicated code: cùng logic ở nhiều nơi -> extract function
// Long method -> tách theo mức trừu tượng (xem PF031)
// Primitive obsession: truyền (lat, lng) lẻ khắp nơi -> gói thành value object Coordinates
// Feature envy: method dùng dữ liệu của object khác nhiều hơn của chính nó -> chuyển method sang object kia

Mỗi smell có một refactoring tương ứng (extract constant/method, introduce parameter object, move method). Điểm cốt lõi: refactoring là chuỗi bước nhỏ, mỗi bước giữ nguyên hành vi, xác nhận bằng test sau mỗi bước — không phải viết lại một mạch.

Vấn đề gặp trong production

Failure mode: refactor không test gây hồi quy. Đây là rủi ro lớn nhất. "Dọn dẹp" một hàm phức tạp mà không có test sẽ vô tình đổi một edge case — một điều kiện biên, một thứ tự xử lý — và bug chỉ lộ ra ở production tuần sau, khó truy về đúng commit refactor. Quy tắc: trước khi refactor code chưa có test, viết characterization test trước — test nắm lại hành vi hiện tại (kể cả hành vi "lạ"), để mọi thay đổi cấu trúc sau đó được kiểm chứng là không đổi hành vi.

// trước khi refactor calculatePrice (không ai dám đụng), khóa hành vi hiện tại lại
test('giữ hành vi hiện tại', () => {
  expect(calculatePrice(sampleCart)).toBe(1234) // ghi lại output thực tế đang chạy
})

Failure mode: refactor lẫn với thay đổi tính năng trong một PR. Trộn "đổi cấu trúc" và "đổi hành vi" trong cùng một thay đổi làm review bất khả thi và khó truy lỗi — không biết một bug đến từ refactor hay từ feature. Tách thành hai PR: một refactor thuần (test xanh không đổi), một feature.

Failure mode: đuổi theo smell mà không có giá trị. Không phải mọi smell đều đáng khử. Một đoạn duplicated nhỏ ở hai chỗ ổn định, hay một magic number rõ trong ngữ cảnh, gắng abstract có khi tạo abstraction sai (gom hai thứ trông giống nhưng thay đổi vì lý do khác — coupling sai). Refactor có chi phí và rủi ro; ưu tiên smell ở code thay đổi thường xuyên và phức tạp, bỏ qua smell ở góc yên tĩnh.

Cách debug và monitor

Trong review, các smell dễ thấy: số lặp lại (magic number), khối code copy-paste (duplication), hàm/class quá lớn, danh sách tham số dài (parameter object), switch/if trên cùng "loại" ở nhiều nơi (thiếu polymorphism). Trước khi đồng ý một PR refactor lớn, câu hỏi đầu tiên: "có test phủ vùng này không?" — nếu không, yêu cầu characterization test trước. Theo dõi độ phủ test ở các module hay refactor; vùng phức tạp mà phủ thấp là nơi refactor nguy hiểm nhất. Công cụ phân tích tĩnh phát hiện duplication và complexity tự động, dùng để khoanh vùng nhưng quyết định khử hay không vẫn là phán đoán.

Tradeoff

Khử smell làm code dễ đọc, dễ sửa, ít lỗi khi thay đổi về sau — đầu tư vào khả năng bảo trì. Cái giá là thời gian, rủi ro hồi quy (đặc biệt khi thiếu test), và nguy cơ tạo abstraction sai khi gom những thứ chỉ tình cờ giống nhau. Quy tắc thực tế: refactor luôn cần test bảo vệ (viết trước nếu chưa có); làm bước nhỏ, mỗi bước test xanh; tách refactor khỏi thay đổi tính năng; và chọn smell để khử theo giá trị — ưu tiên code nóng và phức tạp, chấp nhận smell vô hại ở chỗ ổn định. Refactor không phải mục tiêu tự thân mà là dọn đường cho thay đổi sắp tới.

Câu hỏi phỏng vấn

Code smell là gì, cho vài ví dụ, và vì sao không nên refactor khi chưa có test?

Code smell là dấu hiệu bề mặt gợi ý vấn đề thiết kế sâu hơn — không phải bug, mà là "mùi" cảnh báo code khó sửa và dễ sai khi đổi; ví dụ: duplicated code, long method, magic number, large class, primitive obsession (dùng kiểu nguyên thủy lẻ thay vì gói thành value object), feature envy. Mỗi smell có một refactoring tương ứng (extract constant/method, introduce parameter object, move method). Không nên refactor khi chưa có test vì refactor theo định nghĩa là đổi cấu trúc mà giữ nguyên hành vi — không có test thì không có cách nào biết mình có vô tình đổi một edge case không, và hồi quy sẽ lộ muộn ở production khó truy về commit. Điểm ăn điểm: viết characterization test khóa hành vi hiện tại trước khi đụng code chưa phủ test, làm bước nhỏ test xanh sau mỗi bước, tách refactor khỏi feature trong PR riêng, và chỉ ưu tiên khử smell ở code nóng/phức tạp vì refactor có chi phí và rủi ro tạo abstraction sai.

Hands-on

Lấy một hàm legacy thật chưa có test (kiểu calculatePrice với nhiều điều kiện và magic number mà cả nhóm ngại đụng), viết characterization test khóa lại output hiện tại cho một bộ input đại diện. Sau đó refactor từng bước nhỏ — extract constant cho các magic number, extract method cho khối lặp, gói các tham số nguyên thủy đi cùng nhau thành một parameter object — chạy test sau mỗi bước để xác nhận hành vi không đổi. Cố tình thực hiện một bước làm đổi một edge case và quan sát test bắt được. Cuối cùng, tách một PR đang trộn refactor với thêm tính năng thành hai phần riêng và đối chiếu độ dễ review.