Hannes Schneidermayer Desenvolvedor Java & web

Minhas boas práticas – Geral

1. Comentários – maldição e bênção

Bons comentários são uma arte. Usados com critério, são um presente – mas muitas vezes também são em vão.

Código autodocumentado não significa “sem comentários”. O próprio código deve expressar o como da melhor forma possível; os comentários complementam o porquê, as condições de contorno não óbvias e o contrato de uma API.

A seguir, alguns casos de código autodocumentado, além de comentários bons e ruins.

Um argumento comum é que os comentários acabam ficando desatualizados. Na verdade, esse problema vale igualmente para nomes de variáveis e de métodos!

Ai – nunca faça isso:

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

Autodocumentação: bons nomes de variáveis já resolvem metade do problema

Ruim:

var dateTime = DateTimeUtil.getNow();

Bom:

var now = DateTimeUtil.getNow();

Mais um exemplo:

Ruim:

User user = this.getBruce();

Ruim:

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

Bom:

User userWithoutPermission = this.getBruce();

Autodocumentação: bons nomes de métodos

Ruim:

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

Bom:

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

Autodocumentação: métodos privados

Ruim: o método obriga quem lê a decifrar validação, cálculo e regra de negócio ao mesmo tempo.

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

Bom: o método público conta o fluxo. Métodos privados e stateless dão nome a cada intenção e deixam suas dependências visíveis por meio de parâmetros e valores de retorno.

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

Nem toda linha precisa de um método próprio. O que se extrai é uma ideia coesa que possa ser nomeada com precisão. Já nomes como processData() ou handleStuff() não acrescentam nada.

Regra prática: comente por que algo acontece, e não o que acontece

Está claro o que acontece aqui:

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

Mas por que isso acontece?

Melhor:

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

Outro exemplo da vida real: métodos para mostrar e ocultar o spinner de carregamento do Ajax. Só que são quatro métodos, e o motivo não fica muito claro à primeira vista:

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

Os métodos parecem todos muito semelhantes e, de algum jeito, manipulam a visibilidade via CSS. Com um comentário curto, os casos de uso ficam claros na hora:

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

Mais um exemplo:

Ruim:

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

Bom:

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

Discussão no workshop de 21/06/2021: consenso entre todos os desenvolvedores de que o Javadoc deve, em geral, ser evitado (pesadelo de parâmetros, sempre desatualizado e nada agradável de manter) e só deve ser usado quando um parâmetro realmente precisa de mais explicação ou quando, sem ele, seria preciso ler o método inteiro, por mais longo que seja.

Exceção à regra prática: comente o que acontece – aqui vale a pena

Quando isso facilita muito a leitura, porque de outra forma o código não fica claro:

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

Comentários – palavrões no código do kernel Linux

Para encerrar, uma curiosidade divertida do kernel Linux – a contagem de todos os palavrões no código-fonte:

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

Divirta-se escrevendo comentários!

2. Métodos privados em rotinas grandes são perigosos

Atenção! No mundo orientado a objetos de hoje, sempre existem muitos métodos privados. Normalmente isso não é problema. Passa a ser quando esses métodos acessam campos, ou seja, o estado, e ficam dentro de rotinas grandes. Esses submétodos deveriam ser privados e stateless. Caso contrário, têm a desvantagem de, quando chamados isoladamente, muitas vezes causarem dor de cabeça. Algum desenvolvedor vai ter a ideia de chamar o submétodo. Estados ilegais estão entre as principais causas de bugs. Um artigo interessante sobre isso traz algumas declarações de John Carmack (id Software, Doom) e mostra que, em aplicações nas quais o código precisa ser muito estável (voos espaciais, por exemplo), sub-rotinas muitas vezes são proibidas, o que deixa o código mais determinístico: http://number-none.com/blow/john_carmack_on_inlined_code.html

Essa história só se aplica em parte ao mundo moderno e stateless de Java/Spring, mas mesmo assim é um bom ponto de reflexão.