Best Practices của tôi – Tổng quát

1. Comment – con dao hai lưỡi

Viết comment hay là cả một nghệ thuật. Viết có cân nhắc thì là món quà, nhưng nhiều khi cũng thành công cốc.

Self-documenting Code không có nghĩa là “không có comment”. Bản thân code nên tự thể hiện cách làm càng nhiều càng tốt; comment bổ sung lý do, những ràng buộc không hiển nhiên và hợp đồng (contract) của API.

Dưới đây là một số trường hợp về self-documenting code, cùng với comment tốt và comment tệ.

Một lập luận phổ biến là comment rất dễ bị lỗi thời. Thực ra vấn đề này cũng xảy ra y hệt với tên biến và tên phương thức!

Ối – đừng bao giờ làm thế này:

int age = 32; // set the value of the age integer to 32

Self-documenting: Đặt tên biến có ý nghĩa là đã xong một nửa

Tệ:

var dateTime = DateTimeUtil.getNow();

Tốt:

var now = DateTimeUtil.getNow();

Một ví dụ khác:

Tệ:

User user = this.getBruce();

Tệ:

// testing user without permission
User user = this.getBruce();

Tốt:

User userWithoutPermission = this.getBruce();

Self-documenting: Đặt tên phương thức có ý nghĩa

Tệ:

public String sanitizeInput(String input) {
    // replace underscore with spaces
    return input.replaceAll("_", " ");
}

Tốt:

public String replaceUnderscoreWithSpaces(String input) {
    return input.replaceAll("_", " ");
}

Self-documenting: Phương thức private

Tệ: Phương thức này buộc người đọc phải cùng lúc giải mã phần kiểm tra dữ liệu, phần tính toán và quy tắc nghiệp vụ.

public Invoice createInvoice(Order order, Customer customer) {
    if (order.items().isEmpty()) {
        throw new IllegalArgumentException("Order needs at least one item");
    }

    BigDecimal subtotal = order.items().stream()
            .map(item -> item.unitPrice().multiply(BigDecimal.valueOf(item.quantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
    BigDecimal discount = customer.isPremium()
            ? subtotal.multiply(new BigDecimal("0.10"))
            : BigDecimal.ZERO;

    return new Invoice(order.id(), subtotal, discount, subtotal.subtract(discount));
}

Tốt: Phương thức public kể lại luồng xử lý. Các phương thức private, stateless đặt tên cho từng ý định riêng và thể hiện rõ các phụ thuộc của chúng qua tham số và giá trị trả về.

public Invoice createInvoice(Order order, Customer customer) {
    requireItems(order);
    BigDecimal subtotal = calculateSubtotal(order.items());
    BigDecimal discount = loyaltyDiscountFor(customer, subtotal);

    return invoiceFor(order, subtotal, discount);
}

private static void requireItems(Order order) {
    if (order.items().isEmpty()) {
        throw new IllegalArgumentException("Order needs at least one item");
    }
}

private static BigDecimal calculateSubtotal(List<OrderItem> items) {
    return items.stream()
            .map(item -> item.unitPrice().multiply(BigDecimal.valueOf(item.quantity())))
            .reduce(BigDecimal.ZERO, BigDecimal::add);
}

private static BigDecimal loyaltyDiscountFor(Customer customer, BigDecimal subtotal) {
    return customer.isPremium()
            ? subtotal.multiply(new BigDecimal("0.10"))
            : BigDecimal.ZERO;
}

private static Invoice invoiceFor(
        Order order,
        BigDecimal subtotal,
        BigDecimal discount
) {
    return new Invoice(order.id(), subtotal, discount, subtotal.subtract(discount));
}

Không phải dòng nào cũng cần một phương thức riêng. Thứ nên được tách ra là một ý trọn vẹn, có thể đặt tên chính xác. Ngược lại, những cái tên như processData() hay handleStuff() chẳng mang lại gì.

Quy tắc chung: Comment tại sao điều gì đó xảy ra, thay vì cái gì xảy ra

Ở đây thì rõ ràng cái gì đang xảy ra:

public Duration retryDelay(int retryCount) {
    long seconds = Math.min(1L << retryCount, 60);
    return Duration.ofSeconds(seconds);
}

Nhưng tại sao nó xảy ra?

Tốt hơn:

public Duration retryDelay(int retryCount) {
    // Cap the delay so an outage never blocks queued jobs for more than one minute.
    long seconds = Math.min(1L << retryCount, 60);
    return Duration.ofSeconds(seconds);
}

Thêm một ví dụ thực tế. Đây là các phương thức để hiện và ẩn biểu tượng loading của Ajax. Có điều là có tới bốn phương thức. Lý do thì thoạt nhìn không rõ lắm:

function showAjaxLoader() {
    $('.layout-ajax-loader').children().first().show();
}

function hideAjaxLoader() {
    $('.layout-ajax-loader').children().first().hide();
}

function suppressAjaxLoader() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = 'none';
}

function releaseAjaxLoader() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = '';
}

Các phương thức trông rất giống nhau và có vẻ đều thay đổi khả năng hiển thị qua CSS theo cách nào đó. Chỉ cần một comment ngắn là các trường hợp sử dụng trở nên rõ ràng ngay:

showAjaxLoader: function() {
    $('.layout-ajax-loader').children().first().show();
},

hideAjaxLoader: function() {
    $('.layout-ajax-loader').children().first().hide();
},

// take the control from primefaces and don't allow the spinner to show, no matter what
suppressAjaxLoader: function() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = 'none';
},

// reset the state after hiding, to give the control back to primefaces
releaseAjaxLoader: function() {
    getOnlyElementByClassOrNull('layout-ajax-loader').style.display = '';
},

Một ví dụ khác:

Tệ:

/**
 * Sets the tool tip text.
 *
 * @param text  the text of the tool tip
 */
public void setToolTipText(String text) {}

Tốt:

/**
 * Registers the text to display in a tool tip. The text
 * displays when the cursor lingers over the component.
 *
 * @param text  the string to display. If the text is null,
 *              the tool tip is turned off for this component.
 */
public void setToolTipText(String text) {}

Thảo luận trong buổi workshop ngày 2021-06-21: Tất cả lập trình viên đều đồng ý rằng nhìn chung nên tránh Javadoc (cơn ác mộng tham số, liên tục lỗi thời và khó bảo trì) và chỉ nên dùng khi một tham số thật sự cần giải thích thêm, hoặc khi nếu không có nó thì phải đọc cả một phương thức dài.

Ngoại lệ của quy tắc chung: Comment cái gì xảy ra – ở đây thì nên làm

Khi việc đó giúp code dễ đọc hơn hẳn, vì nếu không thì code sẽ khó hiểu:

let isAlwaysAllowedKey = (
    e.which === 8  || /* BACKSPACE */
    e.which === 35 || /* END */
    e.which === 36 || /* HOME */
    e.which === 37 || /* LEFT */
    e.which === 38 || /* UP */
    e.which === 39 || /* RIGHT*/
    e.which === 40 || /* DOWN */
    e.which === 46 || /* DEL*/
    e.ctrlKey === true && e.which === 65 || /* CTRL + A */
    e.ctrlKey === true && e.which === 88 || /* CTRL + X */
    e.ctrlKey === true && e.which === 67 || /* CTRL + C */
    e.ctrlKey === true && e.which === 90   /* CTRL + Z */
)

Comment – Từ chửi thề trong code của Linux Kernel

Để kết thúc, một giai thoại vui từ Linux Kernel – số lượng từ chửi thề trong toàn bộ mã nguồn:

https://www.vidarholen.net/contents/wordcount/#fuck*,shit*,damn*,idiot*,retard*

Chúc bạn viết comment vui vẻ!

2. Phương thức private trong các routine lớn rất nguy hiểm

Chú ý! Trong thế giới hướng đối tượng ngày nay, lúc nào cũng có rất nhiều phương thức private. Thông thường điều này không thành vấn đề. Nó chỉ trở thành vấn đề khi các phương thức này truy cập field, tức là state, và nằm trong các routine lớn. Những phương thức con như vậy nên là private và stateless. Nếu không, chúng có nhược điểm là khi được gọi riêng lẻ thường gây rắc rối. Sẽ có lập trình viên nảy ra ý định gọi thẳng phương thức con đó. State không hợp lệ là một trong những nguyên nhân chính gây ra bug. Một bài viết thú vị về chủ đề này, tổng hợp một số nhận định của John Carmack (id Software, Doom), cho biết rằng trong những ứng dụng đòi hỏi code cực kỳ ổn định (ví dụ như bay vào vũ trụ), subroutine thường bị cấm, điều này giúp code mang tính tất định hơn: http://number-none.com/blow/john_carmack_on_inlined_code.html

Giai thoại này chỉ áp dụng được một phần cho thế giới Java/Spring stateless hiện đại, nhưng vẫn là một gợi ý đáng suy ngẫm.