Código de má qualidade

É comum deparar-me com falhas nas boas práticas mais básicas de programação. Tais problemas, embora não correspondam necessariamente a bugs, dificultam a leitura do código, o debug e as alterações. São cometidas por programadores com diversos níveis de experiência. Muitas vezes porque algo foi “feito à pressa”, porque não quiseram arriscar um pequeno refactoring, ou porque não puseram em causa a forma mais correta de o fazer.

Poderia dar inúmeros exemplos, mas vou tentar centrar-me nos mais comuns. Este artigo aplica-se a linguagens de alto nível orientadas a objetos (e.g. Java, C#) mas a maior parte aplica-se à maioria das linguagens funcionais.

Se der por si a fazer alguma das seguintes situações, pense duas vezes e aposte num refactoring.

“Copy/paste” de código

A “lei” fundamental a reter é: quanto mais código, mais bugs; por isso pare de copiar código. Ter código repetido no mesmo programa pode resultar em, ao corrigir um bug ou adicionar funcionalidade num lugar, se esquecer de mudar em todos os sítios.
Copiar mais de 1 linha de código devia por si só lançar o alerta. Quase de certeza que:

• se pode isolar a parte repetida (ex: um switch deve apenas conter aquilo que varia);
• se pode construir um procedimento de alto nível parametrizando as diferenças;
• se pode isolar funcionalidade em classes/bibliotecas reutilizáveis;
• entre outros.

Mais de 3 níveis de blocos

if (cond1) {
   if (cond2) {
      for(Classe c : listagem) {
         if (c.getN() > 1) {
            while(...)
         }
      }
   }
}

Este tipo de código além de menos “elegante”, torna a sua leitura extremamente aborrecida e difícil. Pode suceder que (especialmente alguns meses depois), alguém se perca facilmente neste código. A sua leitura obriga a frases como: “estou neste if se X, não Y, após chamar Z, a iterar um array, se K mas não B…”.
Assim, deve evitar-se nesting ao máximo. Quando o nível de nesting é maior ou igual a 3, deverá realizar um refactoring, isolando uma parte da funcionalidade num método auxiliar.

Há até situações em que nem precisa de o fazer. Por exemplo, pode recorrer a syntactic sugar para simplificar uma decisão:

if (estaBomTempo) {
   return "sim";
} else {
   return "não";
}

return estaBomTempo ? "sim" : "não";

Mesmo em setters, esta abordagem pode ser útil (poupa-se 4 linhas):

if (mimetype == null) {
   multimedia.setMimetype("");
} else {
   multimedia.setMimetype(mimetype);
}

multimedia.setMimeType(mimetype == null ? "" : mimetype);

Em métodos em que todo o seu corpo depende de uma condição:

if (tempo != null) {
   // várias linhas de código
}

…. porque não apenas (menos nesting)?

if (tempo == null) {
   return;
}
// várias linhas de código

try/catch(Exception e)

O tratamento de exceções e a tolerância a erros num software é algo extremamente importante mas quase sempre encarado como secundário. Por exemplo, fazer um “try/catch(Exception)” pode ser apenas definido como ‘muito mau’, pois:

• Estamos a desligar o mecanismo de exceções, “abafando” toda e qualquer exceção, até as exceções de Runtime (ex.: NullPointerException); o programa pode acabar num estado incoerente e “estranho”.
• Estamos a tratar da mesma forma toda e qualquer exceção. Cada exceção tem uma natureza (e.g. gravidade) diferente; como tal, deve ser tratada de forma diferente. Algumas são recuperáveis, outras requerem interação do utilizador, outras são apenas sinónimo de bug, etc.

Conclusão: fazer “try/catch(Exception)” é “varrer o lixo todo para baixo do tapete”.

Também não tem lógica nenhuma fazer try/catch(NullPointerException) ou um try/catch(ArraysOutOfBoundsException); deve-se sim testar e resolver as condições que podem originar tais exceções. Deve-se ir à origem do problema evitando-o em vez de o corrigir. As exceções servem para coisas que não se podem evitar.

Quanto a esta temática, haveria muito mais por dizer… contudo, o essencial a ter em mente é que deve encarar o mecanismo de exceções de forma séria.

Por fim, deve sempre manter alguma dissociação entre camadas (decoupling): exemplo clássico: é desvantajoso “abafar um IOException” mas enviá-lo “para cima” também o é, visto que estamos a comprometer as duas camadas (se amanhã se mudar de ficheiros para uma BD, a camada de cima não precisa saber); a minha sugestão é lançar uma exceção nossa (ex.: RepositoryException) e guardar a exceção original.

// no tratamento:
} catch(IOException ex) {
   throw new RepositoryException(ex);
}

// a nossa exceção:
public class RepositoryException extends Exception {
   public RepositoryException(Throwable ex) {
     super(ex);
   }
// ...

Lixo no código

Quanto menos linhas de código (que fazem a mesma coisa) e ainda assim são legíveis, melhor: menos código para manter. Assim:

1. Evite espaços a mais e código não indentado: dificulta a navegação e a compreensão (o NetBeans faz isso sozinho em: Tools > Options > Editor > On Save > Java > Desativar ‘Use All Language Settings’ e selecionar ‘All lines’ em ambas as dropdown boxes);.
2. Apague os “imports” desnecessários (o NetBeans faz isso sozinho em: Tools > Options > Editor > On Save > Java > Ativar ‘Remove Unused Imports’ e ‘Organize Imports’);
3. Declarar uma variável e defini-la separadamente não tem muito sentido:
   StringBuffer resp;
resp = new StringBuffer("teste");
// basta fazer:
StringBuffer resp = new StringBuffer("teste");
Declarar variáveis no início ainda é pior (mesmo em C não faz sentido).
4. Apague os comentários gerados pelo IDE;
5. Não deixe blocos de código comentados pelo caminho… é para isso que serve o controlo de versões…
6. Evite os comentários completamente inúteis; isto é, comentários que apenas dizem o que está bem patente:
   // aqui estou a adicionar o elemento ao vetor
   // este método adiciona os dois resultados
   Não fale de programação mas sim de funcionalidade. É preferível que os nomes dos métodos, funções e variáveis falem por si. Já lá vai o tempo em que os nomes tinham de ter 8 caracteres no máximo. Já agora, chamar “map” ou “array” a algo é inútil. O nome deve refletir o que o elemento contém e se possível, a sua ligação ao negócio (ex. “clients”, “accounts”).
7. “Logs” por todo o lado devem ser evitados. Domine o debugger em vez de “sujar” o código com instruções de debug… Há situações em que é útil, mas a maioria das vezes abusa-se (ex.: “agora entrei aqui”, “adicionando o produto…”). Idealmente, o código deve tender para ter só a parte que “interessa ao negócio”; tudo o que é acessório/auxiliar deve “tender para zero”.
8. Resolva os warnings identificados pelo IDE (é provável que ele tenha razão).
9. Não complique o que é simples:

10. if (xpto <= MAX) {
       return true;
    } else {
       return false;
    }

    return xpto <= MAX;

    ou:

    function Cliente getCliente(int id) {
       Cliente cliente = bd.getCliente(id);
       return cliente;
    }
    

    function Cliente getCliente(int id) {
       return bd.getCliente(id);
    }

Quantidade de código

Tal como ao escrever um texto, se deve ter cuidado com frases e parágrafos muitos extensos, também os métodos e classes de código-fonte devem ter um tamanho sensato. Uma classe com 1000 linhas ou um método com 100 é longe de razoável. De certeza que é possível subdividir, repartir responsabilidades, separar, limpar, etc. É uma questão de bom senso.

Um método/função deve concentrar-se apenas num objetivo, seja ele um algoritmo, tarefa, ou outro. Uma classe também deve ter código que sirva algo em comum. Não deve ser um “saco” de conceitos e operações heterogéneas.

De qualquer forma, ao seguir as recomendações dadas em “Lixo no código”, é bem provável que reduza o tamanho do código em mais de 30%…

Conclusão

Em conclusão, faço um resumo dos sinais que (embora não sejam prova garantida) devem lançar o alerta na mente do programador:

• “Copy/paste” de código dentro do mesmo software;
• Fazer catch e/ou throw de exceções demasiado genéricas (ex.: Exception ou Throwable em Java);
• Fazer catch e/ou throw de exceções de runtime do ambiente (ex.: as RuntimeException do Java, SystemException do C# ou qualquer derivada);
• Abusar do nesting (ter nível 3 é de evitar; nível 4, muito mais);
• Ter um método com mais de 50 linhas;
• Ter uma classe com mais de 1000 linhas;
• Ter tanto código de debug como “de negócio” (mais de 20% já deve lançar o alerta);
• Código que precise de explicações (bom código fala por si; apenas precisa de comentários que o liguem à funcionalidade/negócio);
• Blocos de código comentado;
• Warnings do IDE por resolver.