Hannes Schneidermayer Java- та веброзробник

Мої найкращі практики – загальні

1. Коментарі — прокляття і благословення

Хороший коментар — це мистецтво. Влучний — справжній дар, та часто — зайвий тягар.

Самодокументований код не означає «жодних коментарів». Код має якомога краще сам виражати, як щось працює; коментарі доповнюють його поясненням чому, неочевидними граничними умовами та контрактом API.

Далі — кілька прикладів самодокументованого коду, а також хороших і поганих коментарів.

Поширений аргумент: коментарі швидко застарівають. Насправді ця проблема так само стосується назв змінних і методів!

Ой — ніколи так не робіть:

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

Самодокументований код: влучні назви змінних — половина успіху

Погано:

var dateTime = DateTimeUtil.getNow();

Добре:

var now = DateTimeUtil.getNow();

Ще один приклад:

Погано:

User user = this.getBruce();

Погано:

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

Добре:

User userWithoutPermission = this.getBruce();

Самодокументований код: влучні назви методів

Погано:

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

Добре:

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

Самодокументований код: приватні методи

Погано: метод змушує читача одночасно розбиратися з валідацією, обчисленням і бізнес-правилом.

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));
}

Добре: публічний метод розповідає, як усе відбувається. Приватні stateless-методи називають окремі наміри й роблять свої залежності видимими через параметри та значення, що повертаються.

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));
}

Не кожен рядок потребує окремого методу. Виносити варто цілісну думку, яку можна точно назвати. Назви на кшталт processData() чи handleStuff() натомість нічого не дають.

Емпіричне правило: коментуйте, чому щось відбувається, а не що відбувається

Тут зрозуміло, що відбувається:

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

Але чому це відбувається?

Краще:

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);
}

Ще один приклад із реального життя. Тут показано методи, які показують і приховують Ajax-спінер завантаження. Проте методів чотири, і чому саме так, з першого погляду не зовсім зрозуміло:

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 = '';
}

Усі методи здаються дуже схожими й так чи інакше змінюють видимість через CSS. Короткий коментар одразу прояснює, для чого кожен із них:

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 = '';
},

Ще один приклад:

Погано:

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

Добре:

/**
 * 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) {}

Обговорення на воркшопі 2021-06-21: усі розробники погодилися, що Javadoc загалом варто уникати (кошмар із параметрами, постійно застаріває, незручно підтримувати) і застосовувати лише тоді, коли параметр справді потребує пояснення або коли інакше довелося б читати весь довгий метод.

Виняток із емпіричного правила: коментуйте, що відбувається, — тут це доречно

Коли це помітно полегшує читання, бо інакше код незрозумілий:

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 */
)

Коментарі — лайка в коді ядра Linux

І насамкінець кумедний факт про ядро Linux — кількість лайливих слів у його вихідному коді:

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

Приємного написання коментарів!

2. Приватні методи у великих програмних процедурах небезпечні

Увага! У сучасному об’єктно-орієнтованому світі завжди багато приватних методів. Зазвичай це не проблема. Вона з’являється тоді, коли ці методи звертаються до полів, тобто до стану, і містяться у великих процедурах. Такі допоміжні методи мають бути приватними та stateless. Інакше їхній недолік у тому, що окремий виклик часто завдає клопоту. Розробникам спаде на думку викликати такий допоміжний метод напряму. Неприпустимі стани — одна з головних причин багів. Цікава стаття на цю тему про деякі висловлювання John Carmack (id Software, Doom) і про те, що в застосунках, де код має бути особливо стабільним (наприклад, у космічних польотах), підпрограми часто заборонені, що робить код детермінованішим: http://number-none.com/blow/john_carmack_on_inlined_code.html

Ця історія лише частково стосується сучасного stateless-світу Java/Spring, але все одно дає добру поживу для роздумів.