Desenvolvimento

12 dez, 2013

Revisão de código muda com o tempo

100 visualizações
Publicidade

Começando com revisão de código

No início, os desenvolvedores ajudam uns aos outros, dão uma olhada no código quando alguém pede ou às vezes um desenvolvedor líder ou sênior interfere e revisa o código se estivermos com problemas em testes ou se alguém tiver acabado de entrar na equipe e nós antecipássemos que houvesse algum descuido extra. Também se contrata um especialista para fazer uma revisão segura de código.

Depois que o sistema é lançado, decidimos ser pró-ativos e começamos a fazer revisões baseados no risco: pedimos a qualquer um para escrever código em áreas de alto risco (como framework e security plumbing, APIs, a lógica de negócio core, ou áreas onde tínhamos visto os problemas antes) para obter uma revisão do código. Nós poderíamos ter parado por aí, e teríamos um monte de valor de revisões de código. Mas decidimos continuar, e fazer da análise de código uma prática padrão.

Isso não aconteceu da noite pro dia. Não foi difícil convencer a equipe de que as revisões de código poderiam ajudar – eles já tinham visto bons resultados com base nas avaliações de risco que tínhamos feito. Mas foi difícil mudar a maneira como as pessoas trabalhavam, e para nos certificar de que eles tiveram tempo suficiente para fazer comentários corretamente: tempo para agendar e realizar as avaliações e tempo para compreender e agir sobre o feedback. E também levou um tempo para se chegar a um efetivo processo de revisão de código.

Primeiramente, pedimos aos desenvolvedores para escolherem um amigo e realizarem uma revisão. Os resultados foram misturados. Às vezes, um desenvolvedor poderia estar com alguém legal ou alguém muito ocupado que iria deixá-lo levemente desanimado; ou dois desenvolvedores trocariam informações (eu vou mostrar o meu se você me mostrar o seu) para que eles pudessem obter as revisões o mais rapidamente possível. Porque as pessoas não sabiam quanto de tempo precisavam para ter as revisões concluídas, elas muitas vezes foram deixadas até muito tarde, e feitas depois que o código já havia sido testado e liberado.

E porque a maioria das pessoas não tinha muita experiência em fazer revisões, eles não tinham certeza do que deviam procurar, e como dar feedback significativo. Desenvolvedores ficaram frustrados (“ele me disse para fazê-lo dessa maneira, mas em uma outra mudança, ela me disse que eu deveria fazê-lo de outra”) e por vezes perturbados por críticas negativas.

Finalmente, decidimos que os líderes fariam a maioria das revisões. Embora isso acrescentasse muito mais a carga de trabalho aos líderes, e fez com que eles não pudessem fazer tanto de codificação, ajudou em alguns aspectos. Os desenvolvedores líderes geralmente tinham uma melhor compreensão das necessidades e o que o código deveria fazer do que qualquer outro desenvolvedor que ficou disponível, o que significa que eles tiveram uma melhor chance de encontrar erros reais. E porque a mesma pessoa estava fazendo a maioria de suas reviews, os desenvolvedores receberam feedback consistente.

Como fazemos revisões de código

A forma como fazemos revisões de código se manteve praticamente a mesma ao longo dos anos.

Alterações de código não-triviais são revisadas (seja antes ou depois do check-in), independentemente de quem escreveu o código ou o que o código faz. Nós não realizamos reuniões formais de revisão com um projetor ou impressões, e não achamos necessário o uso de uma ferramenta de avaliação, como Code Collaborator ou Crucible ou as ferramentas internas que o Google ou o Facebook usam para gerenciar e rastrear comentários, embora se tivéssemos adotado uma ferramenta como essa no início poderia ter ajudado o time a começar melhor.

Às vezes, comentários são feitos cara-a-cara, mas na maioria das vezes eles são feitos offline: o revisor e o colaborador trocam informações e talvez arquivos patch por e-mail, porque nós achamos que é mais conveniente e fácil para que todos possam programar (embora as pessoas se reúnam para passar a revisão em detalhes se eles decidirem que é necessário).

O que mudou ao longo do tempo é o que os revisores procuram, e o que eles encontram.

Revisão para correção

Parece que os programadores passam mais tempo discutindo sobre o que procurar em revisões de código do que realmente fazendo revisões de código.

Começamos a fazer revisões de código para melhorar a qualidade do código e encontrar problemas que não estavam sendo pegos no teste. Não como uma maneira de ensinar os desenvolvedores inexperientes como escrever um código melhor, ou uma maneira de difundir o conhecimento sobre como funciona o código para toda a equipe. Esses podem ser os felizes efeitos colaterais, mas revisões devem ser sobre se certificar de que o código funciona direito.

Em vez de usarem listas longas de verificação, os revisores começaram a ter um pequeno conjunto de perguntas quando eles olhavam para código:

Há erros de codificação óbvios ou o código parece suspeito? Existe alguma coisa obviamente ausente ou incompleta? O que tem de ser corrigido para se certificar de que o código funcionará corretamente?

Revisão para correção é um trabalho árduo – um revisor tem que gastar algum tempo para entender os requisitos, e mais tempo pensando sobre o código e tentando entrar na cabeça da pessoa que o escreveu.

Mas, mesmo sem muito tempo ou familiaridade, um bom revisor ainda pode descobrir erros lógicos e omissões, inconsistências (você mudou a, b e d, mas não alterou c. Isso foi de propósito?), codificação comum misturadas (olhando para fora < ao invés de < = ou às vezes > em comparações, por um erro, utilizando as variáveis erradas nos cálculos ou comparações – comprador em vez de vendedor, anfitrião em vez de convidado), depuração do código deixada por acidente, código redundante e checagens que não parecem ser necessárias e outro código suspeito que não parece certo ou é confuso.

Os revisores também podem procurar por questões básicas de segurança (regras de controle de acesso/funções sendo aplicadas corretamente, manuseio correto de dados sensíveis), compatibilidade com versões anteriores e uso adequado de APIs e recursos de linguagem (especialmente para pessoas novas na equipe).

E enquanto nós não pedimos aos revisores para gastarem um bom de tempo revendo os testes automatizados, eles também podem fazer verificações no local para procurar buracos na suíte de teste – especialmente se encontrarem um erro de codificação óbvio (o que significa que alguns testes estão ausentes ou errados).

  • Será que esse código tem a mesma aparência e trabalha do mesmo jeito que o resto da base de código?

Será que segue o estilo e as convenções com os quais a equipe concordou (indentação, cabeçalhos, capitalização e convenções de nomenclatura, sequência de importação…)? Ele usa modelos padrão e regras de formatação automática? Nós não estamos falando de perfeição detalhista aqui, mas não queremos que cada parte do código pareça diferente também.

Convenções arquitetônicas. Será que usa o framework e bibliotecas comuns corretamente? Será que segue corretamente os grandes padrões (MVC etc.) que queremos que as pessoas entendam e usem?

Queríamos desenvolvedores que trabalhassem as questões em torno de coerência e estilo desde o início, enquanto a equipe estava se juntando e todo mundo ainda estava aprendendo e explorando. Isso acabou por demorar mais tempo e causar mais problemas do que esperávamos. Não só é tedioso verificar se há estilo, mas isso também pode ser uma coisa quase religiosa para algumas pessoas. Um desenvolvedor não vai discutir sobre se algo é um bug ou não, pelo menos não depois de ter compreendido o que há de errado e por quê. E a maioria dos desenvolvedores aprecia aprender a usar o framework ou a linguagem de forma mais eficaz, desde que isso seja feito de uma maneira educada e respeitosa. Mas algumas pessoas se feriram gravemente ao longo da estética e estilo, que tipo de código é limpo ou é mais elegante .

Revisores poderiam oferecer outro feedback, claro, conselhos e sugestões sobre a forma de simplificar a solução e melhorar o código, mas deixamos isso para o desenvolvedor quando ou se  agir seguindo esse conselho, desde que o código esteja correto e siga as convenções.

Revisão para entendimento

Com o tempo, o que você procura e o que você obtém de revisões de código devem mudar. Porque o código é alterado. E as pessoas que fazem o trabalho – os desenvolvedores e os revisores – mudam também.

Ficou mais fácil para os revisores encontrarem bugs e código ruim, à medida que os desenvolvedores aprenderam mais sobre como usar os verificadores de código em suas IDEs e depois de encontramos algumas boas ferramentas de análise estática para verificar automaticamente erros comuns de codificação e práticas ruins de codificação. Isso significa que os revisores poderiam gastar seu tempo procurando erros de projeto ou erros de tempo ou erros contábeis de sequência ou erros de concorrência mais importantes, como as condições de corrida e possíveis bloqueios e outros problemas que as ferramentas não conseguem encontrar.

Como a equipe ganhou mais experiência em apoiar o sistema em produção, solucionar e corrigir os problemas e trabalhar com Ops, os revisores começaram a olhar mais de perto a programação defensiva e o tempo de execução de segurança: erro e manipulação de exceção (especialmente as condições de exceção que são difíceis de testar), validação de dados e verificação de limites, limites de tempo e tentativas, aplicar corretamente os contratos de API, e registro e alerta. Certificar-se de que o sistema não vai falhar.

Porque nós resolvemos a maior parte dos problemas de consistência cedo, deixou de ser necessário rever a consistência depois de um tempo. Havia um monte de código, por isso era natural e fácil para os desenvolvedores construírem em cima do que já estava lá, e continuar a seguir o estilo e os padrões vigentes.

Mas isso também significava que havia muito mais código para ser lido e entendido. Então, revisores de código se tornaram mais preocupados com o que fez o código mais difícil de ler e entender. Embora a agitação sobre detalhes como o método e os nomes das variáveis e se as revisões são necessárias ou fazem sentido pode não parecer tão importante, tudo isso se alimenta da correção – avaliadores não podem dizer se o código está correto se não tiverem a certeza de que eles realmente o compreendem.

Os revisores também começaram a olhar mais para a duplicação de código desnecessária porque clones criam mais oportunidades para erros quando você tem que fazer mudanças, e código que não foi atualizado para usar novas bibliotecas ou APIs, ou código que não é necessário em mais nada (especialmente um problema se você usar chaves de recurso e não as remove), ou refatoração que não se terminou ou que saiu do controle.

Compreensão, não crítica

Revisões são sobre a compreensão do código, certificando-se de que ele funciona direito, não de criticá-lo.

É importante ficar de fora de argumentos sobre “o que eu acho que bom código é vs. o que você acha que bom código é”. A revisão nunca deve ser executada ao longo das linhas de “eu sei o que um bom código OO deveria parecer e aparentemente você, não, então deixe-me mostrar-lhe”.

Uma revisão deve ser apenas sobre “Eu tenho que ser capaz de entender esse código bem o suficiente para ter certeza de que ele funciona, e no caso de eu ter que corrigir alguma coisa neste código, mesmo que eu não tivesse escrito desse jeito e desejo que você não tivesse (mas eu vou manter essa parte para mim)”.

Quando revisores ultrapassam essa linha, quando o orgulho e o sentimento de autoestima profissional estão envolvidos, as coisas podem ficar feias rápido.

Além de evitar argumentos, é um desperdício de tempo dar feedback e sugestões para ninguém agir, porque eles estão muito ocupados para chegar a ele ou eles não concordam, ou não é necessário. Se você quiser obter um bom valor de revisões, mantenha-os focados no que é importante.

As principais questões que os revisores tentam responder agora, quando olham para o código são:

  • Será que esse código faz o que deveria fazer? E vai continuar a funcionar, mesmo se algo der errado?

Revisores procuram manter o mesmo tipo de lógica óbvia e problemas funcionais como faziam antes. Mas eles também perguntam: o que acontece se ele for executado em um erro? Ops vai ser capaz de dizer qual era o problema? O que acontece se alguma outra parte do sistema falhar? Será que ele vai tentar novamente e se recuperar, ele vai falhar normalmente, ou será que vai explodir? Qual é a chance de que esse código pegar o lixo passado para ele, e o que aconteceria se isso acontecesse ? E ele pode passar lixo para alguma outra parte do sistema?

  • Posso ter certeza de que esse código faz o que deveria fazer? Eu posso entender o código? Posso acompanhar as mudanças que foram feitas?

É a manutenção de código? Eu poderia corrigi-lo em produção se algo der errado? Existe algum código que também é bonito ou inteligente demais para seu próprio bem? Há alguma coisa óbvia, segura e simples que pode ser feita para tornar o código mais simples e mais fácil de entender e de mudar?

Algumas coisas melhoram com o tempo; outras, não

Ao olhar para as alterações de bases de código ao longo do tempo, Michael Feathers descobriu que a maioria do código raramente ou nunca é modificada quando já está escrito; a maioria das alterações é feita sobre a mesma pequena percentagem da base de código.

Porque revisores continuam vendo o mesmo código que está sendo alterado, ele muda a forma como funciona:

Revisores que viram o código antes não têm que gastar tanto tempo para descobrir o que está acontecendo, eles entendem o contexto e têm uma melhor chance de entender as mudanças e se as mudanças são feitas corretamente.

Ao olhar para o mesmo código novamente, os revisores podem ver problemas antigos que não perceberam antes. Eles devem ser capazes de oferecer feedback mais valioso e menos superficial sobre como o código pode ser simplificado e melhorado.

Há um outro lado do que está sendo feito aqui também, é claro. Os revisores também podem ficar obsoletos ao olharem para o mesmo código, e pararem de olhar de perto o suficiente. E preconceitos do revisor e pontos cegos ficarão amplificados ao longo do tempo.

Como os revisores trabalham com os mesmos desenvolvedores mais e mais, eles vão conhecer melhor uns aos outros. Revisores vão aprender o que procurar, os pontos fortes e fracos do desenvolvedor, tendências do desenvolvedor e preconceitos e pontos cegos, os tipos de erros e omissões que eles tendem a fazer.

Como desenvolvedores conhecem melhor os revisores, o feedback vai ficar mais curto e mais simples, e haverá menos mal-entendidos.

E porque os desenvolvedores saberão mais o que esperar de uma revisão, eles vão começar a verificar o seu próprio código com mais cuidado, e tentar encontrar os problemas antes que o revisor o faça. Isso é o que Jason Cohen no SmartBear chama de “Efeito Ego“: os desenvolvedores escrevem um código melhor porque eles sabem que alguém vai olhar para ele de perto, especialmente se é alguém que respeitam. Algumas pessoas acham que essa é a razão mais importante para fazer revisões de código – para fazer os desenvolvedores trabalharem com mais cuidado, para que eles não pareçam tolos na frente de alguém e pensem sobre como escrever um código que outras pessoas vão ler.

Evitando retornos decrescentes

Como qualquer prática no desenvolvimento de software, você acabará por ver retornos decrescentes de revisões de código, especialmente se você continuar fazendo as mesmas coisas, olhando para os mesmos problemas da mesma maneira. Você pode até chegar a um ponto em que as revisões de código não valem o custo ou o tempo.

Isso não aconteceu com a gente. Continuamos obtendo um monte de valor a partir das revisões, talvez até mais agora.

Mesmo que nossas ferramentas tenham melhorado, à medida que fortalecemos nossa capacidade de teste, continuamos fazendo revisões porque ferramentas de verificação de erros, reviews e testes encontram problemas diferentes e diferentes tipos de problemas. Descobrimos também que revisões tornam o teste mais eficaz e eficiente, pois há menos vai-e-vem entre desenvolvedores e testadores sobre erros que são capturados e corrigidos antes.

E agora que as revisões são uma parte aceita da forma como as pessoas trabalham, não temos que gastar tempo com a venda e revenda do valor de revisões de código para a equipe ou para o negócio. Enquanto as revisões de código estão sendo levadas a sério, e enquanto você se preocupa em escrever um bom código, vale a pena fazê-las.

***

Artigo traduzido pela Redação iMasters, com autorização do autor. Publicado originalmente em http://swreflections.blogspot.com.br/2013/09/code-reviews-change-over-time.html