5 anti-padrões para se evitar na revisão de código

Este artigo é uma tradução/adaptação da publicação da Java Magazine do mês de maio: https://blogs.oracle.com/javamagazine/five-code-review-antipatterns

Revisões de código são essenciais, certo? Mas nem sempre são feitas corretamente. Este artigo aponta e discute alguns anti-padrões específicos que todos nós desenvolvedores provavelmente experimentamos enquanto seu código é revisado ou quando enviam solicitações pull request.

http://slides.com/raphaelcarvalho/code-review/fullscreen#/

01 — Coisas irrelevantes (Nit-Picking)

Imagine o seguinte cenário. O autor do código coloca horas, se não dias, em um esforço para criar a solução que ele pensa ser a melhor possível. Ele considera várias opções de design e segue o caminho que parece ser mais relevante. Ele considera a arquitetura existente e faz alterações nos locais apropriados. Em seguida, envia sua solução para a revisão de código (abre um pull request), e os comentários de especialistas que recebe são:

• “Você deveria estar usando tab, não espaços.”
• “Eu não gosto de onde as chaves estão nesta seção.”
• “Você não tem uma linha em branco no final do seu arquivo.”
• “Suas enumerações estão em maiúsculas e deveriam estar em CamelCase.”

Embora seja importante que o novo código seja consistente com o estilo do código existente, dificilmente isso exige um revisor humano. Os revisores humanos são caros e podem realizar coisas que os computadores não podem. Verificar se os padrões de estilo foram cumpridos é algo que um computador pode facilmente fazer e distrai o real objetivo de uma revisão de código.

Se os desenvolvedores veem muitos desses tipos de comentários durante a revisão de código, isso sugere que a equipe não possui um guia de estilo ou possui um, mas a verificação do estilo não foi automatizada. A solução é usar ferramentas como o checkstyle para garantir que as diretrizes de estilo sejam seguidas ou usar o sonarqube para identificar problemas comuns de qualidade e segurança. Em vez de confiar em revisores humanos para alertar sobre esses problemas, o ambiente de integração contínua pode fazer isso.

Se uma equipe tem uma variedade de estilos de código e não tem como automatizar a verificação do estilo, também é provável que caia na próxima armadilha.

02 — Feedback inconsistente

Às vezes, uma revisão de código pode se transformar em uma discussão entre revisores sobre diferentes abordagens, como se os fluxos ou um loop for clássico sejam os melhores. Como um desenvolvedor deve fazer alterações, fechar a revisão e enviar o código à produção se os membros da equipe tiverem visões opostas no mesmo código?

Mesmo a mente de um único revisor pode mudar facilmente, durante uma revisão ou em uma série de revisões. Em uma revisão, um revisor pode estar pressionando o autor para garantir que uma estrutura de dados com operações de leitura seja usada, enquanto que na próxima revisão, o revisor pode perguntar por que existem várias estruturas de dados para diferentes casos de uso e sugerir que o código seja simplificado, fazendo uma pesquisa linear através de uma única estrutura.

Esse cenário ocorre quando uma equipe não tem uma ideia clara de como são suas “melhores práticas” e quando não descobre quais são suas prioridades, por exemplo:

• O código deveria estar mudando para um estilo mais moderno do Java? Ou é mais importante que o código seja consistente e, portanto, continue usando construções “clássicas” em todos os lugares?
• É importante utilizar @Lazy em todos os relacionamentos das entidades? Ou existem seções em que @Eager é aceitável?

Quase todas as questões de design podem ser respondidas com “depende”. Para ter uma ideia melhor da resposta, os desenvolvedores precisam entender as prioridades de seus sistemas e de suas equipes.

03 — Alterações de design de última hora

O feedback mais desmoralizante que um desenvolvedor pode obter durante a revisão de código é quando um revisor discorda fundamentalmente do design ou arquitetura da solução e força uma reescrita completa do código, gradualmente através de uma série de revisões (consulte a seção a seguir) ou brutalmente rejeitando o código e fazendo o autor recomeçar.

A revisão de código não é o momento certo para revisar o design. Se a equipe estiver seguindo uma revisão clássica de código, o código deverá estar funcionando e todos os testes deverão passar antes da etapa final de outro desenvolvedor ver o código. Nesse momento, horas, dias ou possivelmente semanas (embora eu realmente não espere semanas; as revisões de código devem ser pequenas, mas esse é outro assunto) de esforço para o código em revisão. Declarar durante a revisão do código que o design subjacente está errado é uma perda de tempo de todos.

As revisões de código podem ser usadas como revisões de design, mas se essa for a intenção da revisão de código, a revisão deverá ocorrer no início da implementação. Então, antes que os desenvolvedores avancem demais, eles podem esboçar suas idéias com talvez algumas classes e métodos de stub e alguns testes com nomes e etapas significativos, e talvez enviar algum texto ou diagramas, para que os membros da equipe deem feedback sobre a abordagem a ser adotada.

Se os membros da equipe encontrarem problemas de design genuínos em uma revisão de código final (ou seja, quando o código estiver completo e funcionando), a equipe deverá atualizar seu processo para localizar esses problemas muito mais cedo. Isso pode significar fazer tipos alternativos de revisões, como a sugerida no parágrafo anterior, ideias de quadro branco, programação de pares ou conversar sobre a solução proposta com um líder técnico. Encontrar problemas de design em uma revisão final do código é uma perda de tempo de desenvolvimento e é extremamente desmoralizante para os autores do código.

04 — Comentários como Ping-Pong

Em um mundo ideal, os autores enviam seu código para revisão, os revisores fazem alguns comentários com soluções claras, os autores fazem as alterações sugeridas e reenviam o código, a revisão é concluída e o código, enviado. Mas se isso acontecesse regularmente, quem poderia justificar o processo de revisão de código?

Na vida real, o que costuma acontecer é o seguinte:

1. Uma revisão de código é iniciada
2. Vários revisores fazem várias sugestões: algumas pequenas e fáceis, outras leves sem soluções óbvias e outras complicadas.
3. O autor faz algumas alterações: pelo menos as correções fáceis ou talvez várias alterações em um esforço para fazer os revisores felizes. O autor pode fazer perguntas aos revisores para esclarecer as coisas ou fazer comentários para explicar por que alterações específicas não foram feitas.
4. Os revisores voltam, aceitam algumas das atualizações, fazem outros comentários, encontram outras coisas que não gostam no código original, respondem às perguntas e discutem com outros revisores ou o autor sobre seus comentários na revisão.
5. O autor do código faz mais alterações, adiciona mais comentários e perguntas e assim por diante.
6. Os revisores verificam as alterações, fazem mais comentários e sugestões e assim por diante.
7. Os passos 5 e 6 são repetidos, talvez, para sempre.

Durante o processo, em teoria, as alterações e os comentários devem diminuir para zero até que o código esteja pronto. O caso mais deprimente é quando cada iteração traz pelo menos tantos problemas novos quanto os antigos que foram fechados. Nesse caso, a equipe entrou no “loop infinito de revisão de código”. Isso acontece por vários motivos:

• Isso acontece se os revisores escolherem fornecer feedback inconsistente. Para os revisores que se enquadram nesses hábitos, há um número infinito de problemas a serem identificados e um número infinito de comentários a serem feitos.
• Isso acontece quando não há um objetivo claro para a revisão ou não há diretrizes a serem seguidas durante as revisões, porque todo revisor sente que todos os problemas possíveis devem ser encontrados.
• Isso acontece quando não está claro o que os comentários de um revisor exigem do autor do código. Cada comentário implica uma mudança que deve ser feita? Todas as perguntas implicam que o código não é auto-documentado e precisa ser aprimorado? Ou alguns comentários são feitos simplesmente para educar o autor do código para a próxima vez e são feitas perguntas apenas para ajudar o revisor a entender e aprender?
• Os comentários devem ser entendidos como relevantes ou não relevantes e, se os revisores decidirem que o código precisa ser alterado antes de poder alterar, eles precisam ser explícitos sobre exatamente quais ações o autor do código deve executar.
• Também é importante entender quem é responsável por decidir que a revisão está “concluída”. Isso pode ser conseguido por meio de uma lista de tarefas dos itens que foram verificados e aprovados, ou pode ser realizado por um indivíduo com poderes para dizer “bom o suficiente”. Geralmente, precisa haver alguém que possa romper com os impasses e resolver desacordos. Pode ser um desenvolvedor sênior, um líder ou um arquiteto ou pode até ser o autor do código em equipes nas quais existe um alto grau de confiança. Mas, em algum momento, alguém precisa dizer “a revisão está concluída” ou “quando essas etapas estiverem concluídas, a revisão estará concluída”.

05 — Revisor Fantasma

Aqui é onde eu admito o anti-padrão que eu sou mais culpado de cometer: fantasmas. Seja um revisor ou um autor de código, chega um momento na revisão do código (às vezes logo no início!) em que simplesmente não respondo durante a revisão. Talvez haja um recurso importante ou interessante que me pediram para revisar, por isso decido deixá-lo até um “momento melhor”, quando posso “realmente olhar para ele adequadamente”. Ou talvez a revisão seja grande e eu quero reservar bastante tempo. Ou talvez eu seja o autor e, após uma iteração (ou vinte), não posso mais ler e responder aos comentários, então decido esperar “até que minha cabeça esteja no lugar certo”.

Soa familiar?

Qualquer que seja a causa, às vezes alguém no processo de revisão simplesmente não responde. Isso pode significar que a revisão está morta até que essa pessoa veja o código. Isso é um desperdício: mesmo que alguém tenha investido tempo na criação de um ativo (o novo código), até que esteja em produção, não está agregando valor. Na verdade, provavelmente está apodrecendo à medida que fica cada vez mais atrás do resto da base de código.

Vários fatores podem causar revisões fantasmas. Revisões de código grandes são um fator, porque quem quer examinar dezenas ou centenas de arquivos alterados? Não avaliar as revisões de código como trabalho real ou como parte das entregas é outro fator. Experiências difíceis ou desmoralizadoras de revisão de código são outro fator importante: ninguém deseja parar de codificar (algo que os desenvolvedores geralmente gostam) para participar de uma atividade que consome tempo e destrói a alma.

Aqui estão algumas sugestões para abordar revisões fantasmas:

• Verifique se as revisões de código são pequenas. Cada equipe precisa decidir sua definição sobre isso, mas estará na região de horas ou dias de trabalho para revisar, não semanas.
• Verifique se o objetivo da revisão de código é claro e se o que os revisores devem procurar é claro. É difícil se motivar a fazer algo quando o escopo é “encontrar algum problema possível com o código”.
• Reserve um tempo no processo de desenvolvimento para fazer revisões de código.

Esse último ponto pode exigir disciplina da equipe, ou a equipe pode incentivar a concessão de tempo (por exemplo) recompensando o bom comportamento de revisão de código por meio de objetivos ou de qualquer mecanismo usado para determinar a produtividade de um desenvolvedor.

O que sua equipe pode fazer?

Concentre-se em criar um sólido processo de revisão de código.

Há muitas coisas a considerar ao fazer uma revisão de código e, se os desenvolvedores se preocupassem com todas elas para cada revisão de código, seria quase impossível para qualquer código passar no processo de revisão. A melhor maneira de implementar um processo de revisão de código que funcione para todos é considerar as seguintes perguntas:

• Por que a equipe está fazendo análises? Os revisores têm um trabalho mais fácil quando há um objetivo claramente definido, e os autores do código terão menos surpresas desagradáveis com o processo de revisão.
• O que os membros da equipe estão procurando? Quando existe um objetivo, os desenvolvedores podem criar um conjunto mais focado de coisas para verificar ao revisar o código.
• Quem está envolvido? Quem faz as revisões, quem é responsável pela resolução de conflitos de opinião e quem decide se o código está pronto?
• Quando a equipe faz revisões e quando é concluída? As revisões podem ocorrer iterativamente enquanto os desenvolvedores estão trabalhando no código ou no final do processo. Uma revisão pode durar para sempre se não houver uma orientação clara sobre quando o código está finalmente pronto.
• Onde a equipe faz revisões? As revisões de código não exigem uma ferramenta específica. Portanto, as revisões podem ser tão simples quanto os autores que orientam um colega pelo código em sua mesa.

Depois que essas perguntas forem respondidas, sua equipe poderá criar um processo de revisão de código que funcione bem. Lembre-se, o objetivo de uma revisão deve ser colocar o código em produção, não provar o quão espertos os desenvolvedores são.

Conclusão

Os anti-padrões de revisão de código podem ser eliminados, ou pelo menos atenuados, com um processo claro de revisão de código. Muitas equipes acreditam que deveriam fazer revisões de código, mas não têm diretrizes claras sobre o motivo de fazê-las.

Equipes diferentes precisam de diferentes tipos de revisão de código, da mesma maneira que aplicativos diferentes têm requisitos diferentes de negócios e desempenho. O primeiro passo é descobrir por que a equipe precisa revisar o código e, em seguida, a equipe pode trabalhar das seguintes maneiras:

• Automatizando verificações fáceis (por exemplo, verificando o estilo do código, identificando erros comuns e localizando problemas de segurança)
• Criando diretrizes claras sobre quando uma revisão acontece, o que procurar e quem decide quando as revisões são concluídas
• Tornar as revisões de código uma parte essencial do processo de desenvolvimento

O foco no motivo pelo qual as revisões de código são feitas ajudará as equipes a criar práticas recomendadas para um processo de revisão de código, portanto será muito mais fácil evitar estes anti-padrões.

Créditos: https://blogs.oracle.com/javamagazine/trisha-gee