quarta-feira, 1 de fevereiro de 2012

Um bom programador não comenta


Volta e meia esse assunto vem à tona: comentários no código-fonte. A galera old school defende, mas quem já leu Martin Fowler, ataca! Vamos mostrar nossa opinião com exemplos.





Marreta

Marreta era um termo utilizado antigamente, como sinônimo de "solução de contorno" (ITIL). Era comum ouvir: "fulano marretou o programa para passar". Pois bem, há muitos anos (acho que era 1980), eu trabalhava em uma empresa que usava sistemas feitos em Assembler de Mainframe. Depurar Assembler era como aprender "Japonês em Braile": dificil pra k-4@1h0, Para complicar, havia um programa que tinha o seguinte comentário: "marreta". Só isso.

Bem, meu chefe, que era um idiota (desculpem o pleonasmo), resolveu que o comentário pegava mal e que a tal da "marreta" tinha que ser removida do código-fonte. Pois bem, peguei o deque de cartões, imprimi e fui para a biblioteca (sim, a empresa tinha uma biblioteca!) Fiquei lá por três semanas, estudando a lógica do programa e tentando descobrir onde estava a marreta, pois o comentário não ajudava muito.

Depois de muito tempo perdido, o programador original, que estava aposentado, apareceu por lá. Rapidamente, paguei o almoço do cara. Lá pelas tantas, depois de um monte de chopes, eu perguntei a ele onde estava a tal "marreta" e mostrei a listagem para ele. Depois de rir pra k7 ele me disse que não havia marreta alguma, que ele colocou esse comentário para não pegarem no pé dele.

Granny

O site Java Ranch (http://www.javaranch.com/granny.jsp) tem uma personagem vovó, a Granny, que, entre outras pérolas de sabedoria, diz:

"Debug only code - comments can lie."

Em Português: "Depure apenas o código - os comentários podem mentir."

E realmente é o que acontece: as pessoas mudam o código porém esquecem de mudar os comentários.

Comentários dão falsas ilusões aos desenvolvedores, fazendo-os acreditar que "está tudo bem", "você está no controle" e "tudo vai dar certo". Porém, não tem o menor impacto ou responsabilidade sobre o código-fonte mesmo.

Código comentado != código auto documentado

Ao conversar com outros desenvolvedores, especialmente os mais antigos, noto uma confusão muito grande entre “código auto documentado” e “código comentado”, que são conceitos completamente diferentes. Comentários, além de não documentar o código, podem atrapalhar seu correto entendimento.

A melhor definição de código auto documentado que eu achei foi:


“Código auto documentado é aquele que é legível e facilmente compreendido por um desenvolvedor observador, de modo que suas intenções, propósitos e técnicas são claras, sem necessidade de comentários suplementares.” (Traduzido pelo autor)

John Elm, IBM DeveloperWorks - http://www.ibm.com/developerworks/rational/library/edge/09/jun09/designdebteconomics/


Comentários representam um grande problema no código-fonte pelos seguintes motivos:

a) Representam uma visão pessoal de quem escreveu, e não necessariamente a realidade;
b) Nem sempre são atualizados, quando o código-fonte é alterado;
c) Dão a falsa ilusão de que o código é facilmente inteligível;
d) Podem enganar os outros!

Isto quer dizer que não devemos comentar o código-fonte? Não! Existem os comentários bons e os malévolos. Por exemplo, comentários que explicam classes e métodos, como os do estilo JavaDoc (Oracle JavaDoc) devem sempre serem escritos, assim, fica fácil descobrir o uso de uma classe sem ter que analisar seu código-fonte. Porém, comentários com intenção de explicar o código-fonte devem ser evitados.

Martin Fowler diz que os comentários são indícios de "maus cheiros" no código-fonte: http://wiki.java.net/bin/view/People/SmellsToRefactorings, pelos seguintes motivos: podem rapidamente se tornarem extensos e reduzem a clareza do código, apontando alguns refactorings para evitar o seu uso:  "Extract Method", "Rename Method" e "Introduce Assertion".

Como evitar comentários?

Martin Fowler (FOWLER), faz uma pergunta ineressante: se você sente necessidade de comentar um algoritmo complexo, por que não o reescreve para torná-lo mais inteligível? Ele introduz os refactorings, técnicas de rearranjo de código-fonte, que podem facilitar o seu entendimento. Entre eles:


Nada como um exemplo...

Vamos ver um pequeno exemplo. Imagine o código de uma classe que representa um empréstimo bancário. Ela possui um método para calcular a taxa de Juros a ser cobrada em um financiamento. Vamos ver o método que calcula a taxa de juros:

package com.galaticempire.teste;

import java.util.List;
public class Emprestimo {
  ...
  public double calcularTaxa (Cliente cliente, double montante, 
    int meses, int tipo) {
 double tusar = lerTaxaPadrao();
 if (meses < 6 && (tipo == 2 || tipo == 4)) {
  tusar = tusar * calcPercEcp();
 }
 switch(tipo) {
 case 1:
  tusar = calcTxc();
  if (meses < 2) {
   tusar = tusar * 0.80d;
  }
  break;
 case 2:
  if (cliente.lcredcli < montante) {
   tusar = tusar * tcalcprcli(cliente);
  }
  break;
 case 3:
  if (cliente.emprestimos.size() > 0) {
   for (Emprestimo emp : cliente.emprestimos) {
    if (emp.getTipo() == 3) {
     tusar = tusar * 1.15d;
    }
   }
  }
 }
 return tusar;
  }
 ...
}

Depois, uma outra classe é responsável por criar novos empréstimos. Eis um trecho do seu código-fonte:

Emprestimo emprestimo = new Emprestimo (cliente, meses, montante, tipo);
emprestimo.taxa = emprestimo.calcularTaxa(cliente, montante, meses, tipo);
...


O cálculo da taxa está meio difícil de entender, não? Seria “legal” se contivesse alguma explicação sobre os critérios e cálculos. O programador também pensou assim, logo colocou alguns comentários para explicar como o cálculo funciona:

public double calcularTaxa 
 (Cliente cliente, double montante, 
   int meses, int tipo) {
  
/*
 * Calcula a taxa a ser usada no próprio emprestimo. Tem 4 tipos:
 * 1=consignado com desconto em folha
 * 2=normal
 * 3=emergencial. Se tiver mais de 1, aumenta a taxa em 15%
 * 4=especial
 * ele pega a taxa padrão do banco de dados, mas pode aumentar ou
*  diminuir
 * de acordo com outros parâmetros.  
 *
 */
  double tusar = lerTaxaPadrao(); // Lê a taxa padrão do banco de dados
  if (meses < 6 && (tipo == 2 || tipo == 4)) {
 // empréstimo de curto prazo, adicionar percentual à taxa padrão
 tusar = tusar * calcPercEcp();
  }
  switch(tipo) {
    case 1:
 tusar = calcTxc(); // é consignado use a taxa consig 
 if (meses < 2) {
   tusar = tusar * 0.80d; // Se menor que 2 meses, dê um desconto
 }
 break;
    case 2:
 // Empréstimo normal
 if (cliente.lcredcli < montante) {
   // cliente pede mais que o limite
   tusar = tusar * tcalcprcli(cliente); // calculo da tx risco
 }
 break;
    case 3:
 if (cliente.emprestimos.size() > 0) {
  // ver se já tem outro emergencial. Se tiver, aumenta taxa em 15%
   for (Emprestimo emp : cliente.emprestimos) {
      if (emp.getTipo() == 3) {
  tusar = tusar * 1.15d;
  break;
      }
   }
        }
      // tipo 4 é comum usar tx padr 
  }
 return tusar;
}



Melhorou alguma coisa? Bem, acho que não... Agora, além de ler o código, tenho que ler e entender os comentários...

Para demonstrar a ineficácia dos comentários, imagine que o requisito foi alterado e agora o critério para empréstimos emergenciais mudou. Ao invés de aplicar multa de 15% (para quem tem mais de um empréstimo deste tipo), o programa deverá verificar a quantidade de empréstimos emergenciais que o cliente tem. Se o cliente tiver mais do que dois empréstimos emergenciais, então a função deverá obter o percentual a ser aplicado à taxa. Eis a alteração efetuada:


case 3:
  if (cliente.emprestimos.size() > 0) {
  // ver se já tem outro emergencial. Se tiver, aumenta taxa em 15%
    int conta = 0;
    for (Emprestimo emp : cliente.emprestimos) {
 if (emp.getTipo() == 3) {
   conta++;
 }
    }
    if (conta > 2) {
 tusar = tusar * tcalctxempemadic(conta);
    }
  }


Legal, não? Testado e funcionando! Só tem um problema: o programador não alterou o comentário!
Isto é exatamente o que a “granny” (http://www.javaranch.com/granny.jsp), do Java Ranch diz: “debug only code: comments can lie!” (depure apenas o código, os comentários podem mentir). Muita gente vai argumentar que sempre revisa os comentários, e que isto é muito difícil de acontecer, mas sejamos francos: o código está fácil de alterar? Não está acontecendo nada errado?

A classe viola o princípio da responsabilidade única


Para começar, o que o cálculo da taxa está fazendo dentro da classe “Empréstimo”? O cálculo é uma regra de negócio e não uma responsabilidade específica da classe “Empréstimo”, certo? O problema é que a classe “Empréstimo” é considerada como parte do modelo, logo, existem várias classes que dependem dela. Uma alteração na classe “Empréstimo” sujeita todas as outras classes a possíveis problemas.

O princípio da responsabilidade única (“Single Responsability Principle”) diz que cada responsabilidade que uma classe assume é uma razão para alterações. Logo, uma classe deve ter uma e somente uma razão para ser alterada. Não alterar a classe “Empréstimo” só porque a regra de cálculo da taxa mudou, afinal de contas, só serão afetados os novos empréstimos e não os antigos.

Acoplamento de controle entre métodos


Na verdade, o método “calcularTaxa” está sendo controlado externamente, logo, há um acoplamento de controle com quem o invoca. Isto é devido ao uso do condicional baseado no parâmetro. Na verdade, existem várias regras dependendo do tipo de empréstimo:

  1. Empréstimo consignado, que utiliza a taxa de consignação, com um possível desconto, se for de curto prazo;
  2. Empréstimo normal, que usa a taxa padrão, mas pode ter um percentual de risco;
  3. Empréstimo emergencial, que está sujeito à quantidade que o cliente já possui;
  4. Empréstimo comum, que simplesmente usa a taxa padrão com o desconto para curto prazo;

Notamos que a diferença entre o “Empréstimo normal” e o “Empréstimo comum” é a ausência do percentual de risco. Não é difícil perceber que esta parte do sistema é muito sensível à política da empresa, logo, deve sofrer alterações com maior frequência. Poderíamos utilizar o refactoring: “Substituir condicional por polimorfismo”, proposto por Fowler (FOWLER). Neste caso, criaríamos uma classe padrão “RegraDeEmprestimo” e classes filhas, para cada tipo de empréstimo.

Só que não é exatamente o caso. As regras nada têm em comum. Podemos criar uma interface comum para cada tipo de regra, e instanciar de acordo com o tipo de empréstimo. Podemos usar um padrão “Strategy” para escolher qual regra utilizar. Eis a interface comum a todas as regras de cálculo de taxa de empréstimo:

public interface RegraDeEmprestimo {
 public double calcularTaxa(Cliente cliente, Emprestimo emprestimo);
}

Agora, vamos ver como ficaria a classe “EmprestimoNormal”:

package com.galaticempire.teste;

import static com.galaticempire.teste.Constantes.*;

public class EmprestimoNormal implements RegraDeEmprestimo {

  @Override
  public double calcularTaxa(Cliente cliente, Emprestimo emprestimo) {
     double taxaCalculada = lerTaxaPadrao(); 
     if (emprestimo.meses < CURTO_PRAZO_NORMAL) {
       taxaCalculada *= calcularPercentualCurtoPrazo();
     }
     double limiteDeCreditoDoCliente = cliente.lcredcli;
     if (limiteDeCreditoDoCliente < emprestimo.montante) {
       taxaCalculada *= obterTaxaDeRiscoDoCliente(cliente);
     }
     return taxaCalculada;
  }

  private double obterTaxaDeRiscoDoCliente(Cliente cliente) {
    double taxaDeRisco = 1.0d; 
    // obter ou calcular a taxaDeRisco daquele cliente...
    return taxaDeRisco;
  }

  private double calcularPercentualCurtoPrazo() {
    double percentualCurtoPrazo = 1.0d;
    // Obter o calcular o percentual curto prazo...
    return percentualCurtoPrazo;
  }

  private double lerTaxaPadrao() {
    double taxaPadrao = 0.0d;
    // Ler e/ou calcular a taxa padrão...
    return taxaPadrao;
  }

}

Eu empreguei alguns refactorings aqui, como: “Introduzir variável explicativa”, pois nós temos acesso à classe “Empréstimo”, mas não à classe “Cliente”, logo, não podemos trocar o nome horrível daquela propriedade “lcredcli”, mas trocamos o nome de outros métodos originalmente utilizados na classe “Empréstimo”.

Você pode alegar que, como o método tem um “if”, ainda existe um condicional, logo, o refactoring não foi perfeito. Bem, condicionais sempre vão existir. O problema ocorre quando um método muda radicalmente o seu comportamento dependendo de um parâmetro, como o que ocorria anteriormente, denotando acoplamento “de controle”.

Agora, vamos ver a classe para regra de empréstimo consignado:

package com.galaticempire.teste;

import static com.galaticempire.teste.Constantes.*;

public class EmprestimoConsignado 
  implements RegraDeEmprestimo {

  @Override
  public double calcularTaxa(Cliente cliente, Emprestimo emprestimo) {
     double taxaCalculada = lerTaxaConsignada(); 
     if (emprestimo.meses < CURTO_PRAZO_CONSIGNADO) {
      taxaCalculada *= REDUCAO_CURTO_PRAZO_CONSIGNADO;
     }
     return taxaCalculada;
  }

  private double lerTaxaConsignada() {
     double taxaConsignada = 0.0d;
     // Ler e/ou calcular a taxa para empréstimos consignados...
     return taxaConsignada;
  }

}
Repare que criamos mais duas constantes, reforçando nosso princípio de evitar literais a qualquer custo. Eis a classe para empréstimos emergenciais:

package com.galaticempire.teste;
import static com.galaticempire.teste.Constantes.*;
public class EmprestimoEmergencial 
  implements RegraDeEmprestimo {

  @Override
  public double calcularTaxa(Cliente cliente, Emprestimo emprestimo) {
     double taxaCalculada = lerTaxaPadrao(); 
     int QtdEmprestimosEmergenciais = 0;
     for (Emprestimo emprestimoCliente : cliente.emprestimos) {
        if (emprestimoCliente.getTipo() == EMPRESTIMO_EMERGENCIAL) {
          QtdEmprestimosEmergenciais++;
        }
     }
     if (QtdEmprestimosEmergenciais > LIMITE_QTD_EMERGENCIAIS) {
        taxaCalculada = taxaCalculada 
          * calcularAcrescimoTaxa(QtdEmprestimosEmergenciais);
     }
     return taxaCalculada;
  }

  private double lerTaxaPadrao() {
     double taxaPadrao = 0.0d;
     // Ler e/ou calcular a taxa padrão...
     return taxaPadrao;
  }

  private double calcularAcrescimoTaxa(int qtdEmprestimosEmergenciais) {
     double acrescimo = 1.0d; 
     // obter ou calcular o acréscimo para quem tem mais que o limite
     return acrescimo;
  }
}

Aqui comecei a notar uma duplicidade de código que está me incomodando: o método “lerTaxaPadrao()” está replicado. Vamos pensar nele mais tarde. Agora veremos a classe para empréstimos comuns:

package com.galaticempire.teste;

import static 
 com.galaticempire.teste.Constantes.CURTO_PRAZO_NORMAL;
public class EmprestimoComum implements RegraDeEmprestimo {
  @Override
  public double calcularTaxa(Cliente cliente, Emprestimo emprestimo) {
     double taxaCalculada = lerTaxaPadrao(); 
     if (emprestimo.meses < CURTO_PRAZO_NORMAL) {
        taxaCalculada *= calcularPercentualCurtoPrazo();
     }
     return taxaCalculada;
  }

  private double calcularPercentualCurtoPrazo() {
     double percentualCurtoPrazo = 1.0d;
     // Obter o calcular o percentual curto prazo...
     return percentualCurtoPrazo;
  }

  private double lerTaxaPadrao() {
     double taxaPadrao = 0.0d;
     // Ler e/ou calcular a taxa padrão...
     return taxaPadrao;
  }
}

Mais uma vez, noto a duplicidade, pois temos os métodos “lerTaxaPadrao()” e “calcularPercentualCurtoPrazo()”. Podemos tomar duas atitudes:

  • Substituir a interface comum por uma classe abstrata, implementando os métodos duplicados nela;
  • Criar uma outra classe e mover estes métodos para ela;

A primeira vista, substituir a interface por classe abstrata parece uma boa ideia, mas não é... Nem todas as regras usariam estes métodos, logo, eles não representam um comportamento comum. Melhor seria criar uma outra classe e utilizar o padrão “Delegate” para delegar a execução para este classe específica. Esta solução é a melhor. Vamos ver como ficaria esta classe:

package com.galaticempire.teste;

public class RegrasAuxiliares {
 public static double calcularPercentualCurtoPrazo() {
  double percentualCurtoPrazo = 1.0d;
  // Obter o calcular o percentual curto prazo...
  return percentualCurtoPrazo;
 }

 public static double lerTaxaPadrao() {
  double taxaPadrao = 0.0d;
  // Ler e/ou calcular a taxa padrão...
  return taxaPadrao;
 }
}

Eu optei por criar os métodos estáticos, mas isto dependerá de sua função. Agora, é só delegar em cada classe, vou mostrar como ficaria na classe “EmprestimoNormal”:

package com.galaticempire.teste;

import static com.galaticempire.teste.Constantes.*;
import com.galaticempire.teste.RegrasAuxiliares;;
public class EmprestimoNormal 
implements RegraDeEmprestimo {
  @Override
  public double calcularTaxa(Cliente cliente, Emprestimo emprestimo) {
     double taxaCalculada = lerTaxaPadrao(); 
     if (emprestimo.meses < CURTO_PRAZO_NORMAL) {
        taxaCalculada *= calcularPercentualCurtoPrazo();
     }
     double limiteDeCreditoDoCliente = cliente.lcredcli;
     if (limiteDeCreditoDoCliente < emprestimo.montante) {
        taxaCalculada *= obterTaxaDeRiscoDoCliente(cliente);
     }
     return taxaCalculada;
  }

  private double obterTaxaDeRiscoDoCliente(Cliente cliente) {
     double taxaDeRisco = 1.0d; 
     // obter ou calcular a taxaDeRisco daquele cliente...
     return taxaDeRisco;
  }

  private double calcularPercentualCurtoPrazo() {
     return RegrasAuxiliares.calcularPercentualCurtoPrazo();
  }

  private double lerTaxaPadrao() {
     return RegrasAuxiliares.lerTaxaPadrao();
  }
}

Note que eu não removi os métodos locais da classe, apenas deleguei a execução para a classe auxiliar. A diferença pode ser pequena, mas me dá a chance de executar algum procedimento, mesmo que seja para gerar log.

Finalmente, a classe que originalmente gerava o empréstimo será alterada para usar o algoritmo mais apropriado:

Emprestimo emprestimo = new Emprestimo (cliente, meses, 
   montante, tipoEmprestimo);
RegraDeEmprestimo regra = null;
switch (tipoEmprestimo) {
 case EMPRESTIMO_NORMAL:
  regra = new EmprestimoNormal();
  break;
 case EMPRESTIMO_COMUM:
  regra = new EmprestimoComum();
  break;
 case EMPRESTIMO_CONSIGNADO:
  regra = new EmprestimoConsignado();
  break;
 case EMPRESTIMO_EMERGENCIAL:
  regra = new EmprestimoEmergencial(); 
}
emprestimo.taxa = regra.calcularTaxa(cliente, emprestimo);

Na verdade, subimos o condicional de nível de abstração, para um local onde ele é mais aceitável. O código-fonte todo está mais claro e simples de entender e alterar, dispensando comentários. Para falar a verdade, os únicos comentários que deixei foi porque não quis entrar em detalhes do código, como nas funções “lerTaxaPadrao()” e “obterTaxaDeRiscoDoCliente()”, para dar alguns exemplos.

Todo refactoring tem que ter um limite, que é estabelecido quando o esforço não compensa o benefício. Se o condicional te incomoda muito, há várias soluções, como criar um "injetor" para a classe de regras. Ele receberia o tipo de empréstimo e o usaria para ler o nome da classe de um banco de dados ou de um arquivo de propriedades, sem ter que se referenciar às classes concretas diretamente, por exemplo:

regra = injetarRegra(tipoEmprestimo);
...


public RegraDeEmprestimo injetarRegra(tipoEmprestimo) {
    // lê o nome da classe concreta 
    //fazendo uma "query" com o "tipoEmprestimo"
    RegraDeEmprestimo regra = Class.forName(nome).newInstance();
    return regra;
}

Porém, você deve avaliar se é realmente necessário chegar a esse nível de indireção.

O mais importante é a lição que vai ficar. Eu não tive este trabalho todo só para evitar os comentários. Na verdade, a necessidade de comentários é sinal de mau cheiro no código, e precisa ser tratada. O resultado é que o código ganhou em manutenibilidade.


Pô, mas você trocou uma classe por várias, aumentando o código!

Sim. É claro. Refactoring, geralmente, tem esse efeito.

Porém, conseguimos dois grandes benefícios:

  1. O código-fonte ficou mais fácil de entender, dispensando comentários;
  2. Diminuímos a vulnerabilidade da classe "Empréstimo" a alterações. Agora, as alterações de políticas de juros serão feitas apenas nas classes específicas.
É claro que este exemplo é bem limitado e não mostra totalmente o poder do refactoring, mas é um exemplo de como podemos tornar o código mais simples, evitando ter que adicionar comentários explicativos.

Comentários devem servir para a interface da classe ou componente, e não para documentar o código-fonte.