Moje osvědčené postupy – obecné

1. Komentáře – prokletí i požehnání

Dobrý komentář je umění. Promyšlený potěší, často ale nic neřeší.

Samodokumentující kód neznamená „žádné komentáře“. Kód má co nejlépe sám vyjadřovat, jak věci fungují; komentáře doplňují proč, neočividné okrajové podmínky a kontrakt API.

Následuje několik případů samodokumentujícího kódu a také dobrých a špatných komentářů.

Častým argumentem je, že komentáře rychle zastarávají. Tento problém ale stejně tak platí pro názvy proměnných a metod!

Au – tohle nikdy nedělejte:

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

Samodokumentující kód: smysluplné názvy proměnných jsou půl úspěchu

Špatně:

var dateTime = DateTimeUtil.getNow();

Dobře:

var now = DateTimeUtil.getNow();

Další příklad:

Špatně:

User user = this.getBruce();

Špatně:

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

Dobře:

User userWithoutPermission = this.getBruce();

Samodokumentující kód: smysluplné názvy metod

Špatně:

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

Dobře:

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

Samodokumentující kód: privátní metody

Špatně: metoda nutí čtenáře rozluštit validaci, výpočet i obchodní pravidlo najednou.

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

Dobře: veřejná metoda vypráví, jak celý postup probíhá. Privátní bezstavové metody pojmenovávají jednotlivé záměry a jejich závislosti jsou vidět v parametrech a návratových hodnotách.

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

Ne každý řádek potřebuje vlastní metodu. Vyčleňuje se ucelená myšlenka, kterou lze přesně pojmenovat. Názvy jako processData() nebo handleStuff() naproti tomu nic nepřinášejí.

Základní pravidlo: komentujte, proč se něco děje, ne co se děje

Je jasné, co se tu děje:

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

Ale proč se to děje?

Lépe:

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

Další příklad z praxe. Jsou tu metody, které zobrazují a skrývají ajaxový indikátor načítání. Jenže metody jsou čtyři. Proč, to na první pohled není úplně jasné:

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

Všechny metody vypadají velmi podobně a nějak mění viditelnost přes CSS. Krátký komentář hned objasní, k čemu která slouží:

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

Další příklad:

Špatně:

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

Dobře:

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

Diskuse na workshopu 2021-06-21: Všichni vývojáři se shodli, že Javadocu je obecně lepší se vyhnout (noční můra s parametry, neustále zastarává a špatně se udržuje) a používat ho jen tehdy, když parametr opravdu potřebuje bližší vysvětlení, nebo když by člověk jinak musel číst celou dlouhou metodu.

Výjimka ze základního pravidla: komentujte, co se děje – tady je to na místě

Když to výrazně usnadní čtení, protože kód by jinak nebyl srozumitelný:

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

Komentáře – sprostá slova v kódu linuxového jádra

Na závěr ještě veselá historka z linuxového jádra – počet všech sprostých slov ve zdrojovém kódu:

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

Ať vás psaní komentářů baví!

2. Privátní metody ve velkých programových rutinách jsou nebezpečné

Pozor! V dnešním objektově orientovaném světě je vždy spousta privátních metod. To obvykle nevadí. Problém nastává teprve tehdy, když tyto metody přistupují k polím, tedy ke stavu, a jsou součástí velkých rutin. Takové pomocné metody by měly být privátní a bezstavové. Jinak mají tu nevýhodu, že samostatné zavolání často nadělá potíže. Vývojáře totiž napadne pomocnou metodu zavolat. Nepovolené stavy jsou jednou z hlavních příčin chyb. Zajímavý článek na toto téma o některých výrocích Johna Carmacka (id Software, Doom) a o tom, že v aplikacích, kde musí být kód velmi stabilní (například v kosmonautice), jsou podprogramy často zakázané, což kód činí determinističtějším: http://number-none.com/blow/john_carmack_on_inlined_code.html

Tuto historku lze na moderní bezstavový svět Javy a Springu uplatnit jen částečně, přesto je to dobrý podnět k zamyšlení.