Como posso sugerir com tato melhorias no código mal projetado de outras pessoas durante a revisão?

Acredito muito no código limpo e no artesanato de código, embora atualmente esteja em um trabalho em que isso não seja considerado uma prioridade. Às vezes, me encontro em uma situação em que o código de um parceiro é repleto de design confuso e pouca preocupação com manutenção futura, embora seja funcional e contenha pouco ou nenhum erro.

Como sugerir melhorias em uma revisão de código quando você acredita que há muita coisa que precisa mudar e que há um prazo a chegar? Lembre-se de que sugerir que as melhorias sejam feitas após o prazo final pode significar que elas serão totalmente priorizadas quando novos recursos e correções de erros surgirem.

1. Verifique novamente sua motivação. Se você acha que o código deve ser alterado, deve ser capaz de articular algum motivo pelo qual pensa que deve ser alterado. E esse motivo deve ser mais concreto do que "eu teria feito diferente" ou "é feio". Se você não pode apontar para algum benefício resultante da alteração proposta, não faz muito sentido gastar tempo (também conhecido como dinheiro) em alterá-la.

2. Toda linha de código no projeto é uma linha que precisa ser mantida. O código deve ter o tempo necessário para realizar o trabalho e ser facilmente compreendido, e não mais. Se você pode encurtar o código sem sacrificar a clareza, isso é bom. Se você pode fazê-lo enquanto aumenta a clareza, é muito melhor.

3. O código é como concreto: é mais difícil mudar depois de um tempo. Sugira suas alterações com antecedência, se puder, para que o custo e o risco das mudanças sejam minimizados.

4. Toda mudança custa dinheiro. Reescrever o código que funciona e é improvável que precise ser alterado pode ser um esforço desperdiçado. Concentre sua atenção nas seções que estão mais sujeitas a alterações ou que são mais importantes para o projeto.

5. A forma segue a função e, às vezes, vice-versa. Se o código estiver confuso, é mais provável que ele também contenha bugs. Procure esses erros e critique a funcionalidade defeituosa, em vez do apelo estético do código. Sugira melhorias que tornam o código melhor e facilitam a verificação da operação do código.

6. Diferencie entre design e implementação. Uma classe importante com uma interface ruim pode se espalhar por um projeto como o câncer. Isso não apenas diminuirá a qualidade do restante do projeto, mas também aumentará a dificuldade de reparar os danos. Por outro lado, uma classe com uma interface bem projetada, mas com uma péssima implementação não deve ser um grande problema. Você sempre pode reimplementar a classe para obter melhor desempenho ou confiabilidade. Ou, se funcionar corretamente e for rápido o suficiente, você pode deixá-lo em paz e sentir-se seguro ao saber que seu cruft está bem encapsulado.

Para resumir todos os pontos acima: Verifique se as alterações propostas agregam valor.

Há um ponto ideal para agregar valor através da refatoração. As mudanças precisam realizar três coisas:

• melhorar o código que provavelmente mudará
• aumentar a clareza
• custa menos esforço

Considerações:

1. Sabemos que o código limpo é mais barato de escrever e manter, e é mais divertido trabalhar. Seu trabalho é vender essa ideia para as pessoas da sua empresa. Pense como um vendedor, não como um rabugento arrogante (ou seja, não como eu).
2. Você não pode vencer, só pode perder menos.
3. Concentre-se em agregar valor real - não apenas beleza. Eu gosto que meu código tenha uma boa aparência, mas às vezes tenho que aceitar que isso é mais barato.
4. Uma boa maneira de encontrar o ponto ideal é seguir o Princípio dos Escoteiros - quando você trabalha em uma área de código, sempre a deixe em melhor forma do que a encontrada.
5. Uma pequena melhoria é melhor que nenhuma melhoria.
6. Faça bom uso de ferramentas automatizadas. Por exemplo, ferramentas que apenas limpam um pouco a formatação podem fazer muita diferença.
7. Venda outras idéias que, aliás, melhoram a clareza do código. Por exemplo, o teste de unidade incentiva a decomposição de métodos grandes em métodos menores.

Se o código funcionar sem erros sérios, e um prazo final importante (como efeitos P&L ou PR corporativo) for iminente, é tarde demais para sugerir melhorias que exijam grandes mudanças. Mesmo melhorias no código podem criar riscos para a implantação do projeto. O tempo para melhorias foi no início do projeto, quando havia mais tempo para investir na robustez futura da base de código.

A revisão de código tem três propósitos:

1. Verificando erros

2. Verificando onde o código pode ser aprimorado

3. Ferramenta de ensino para quem escreveu o código.

A avaliação da qualidade do design / código é obviamente sobre os nºs 2 e 3.

Até o ponto 2:

• Deixe MUITO claro quais são os benefícios das alterações propostas versus os custos a serem corrigidos. Como qualquer decisão de negócios, isso deve ser sobre análise de custo / benefício.

  Por exemplo, "a abordagem" X do design reduziria significativamente a probabilidade de o erro Y ocorrer ao fazer a alteração Z, e sabemos que esse código sofre alterações do tipo Z a cada 2 semanas. O custo de lidar com a interrupção da produção do bug Y + encontrar o bug + consertar e liberar o conserto + custo de oportunidade por não entregar o próximo conjunto de talentos é $A; enquanto o custo de limpar o código agora e o custo de oportunidade (por exemplo, preço do envio atrasado ou com menos recursos) é $B. Agora, avalie - ou melhor, peça ao seu líder / gerente de equipe - avalie $Avs $Be decida.

  • Isso ajudará a equipe inteligente a gerenciar efetivamente isso. Por exemplo, eles tomarão uma decisão racional usando informações COMPLETAS

  • Isso (especialmente se você exprimir isso bem) aumentará SEU status - por exemplo, você é alguém inteligente o suficiente para ver os benefícios de um design melhor, e inteligente o suficiente para não exigi-lo religiosamente sem considerar as considerações de negócios.

  • E, no provável evento do bug Z, você ganha muito mais força no próximo conjunto de sugestões.

Até o ponto 3:

• MUITO claramente delineado "deve corrigir" bugs / problemas de "Esta é uma prática recomendada e realmente deve ser corrigida, se pudermos poupar recursos - consulte a análise pró / contra em anexo" aprimoramentos de design (anexar o descrito no item 2 acima) vs " Essas são diretrizes gerais que, acredito, ajudariam a melhorar a robustez do código, para que você possa manter o código com mais facilidade "alterações opcionais. Observe a redação - não se trata de "tornar seu código como o que eu quero" - é "se você fizer isso, você obtém os benefícios a, b, c". O tom e a abordagem são importantes.

Escolha suas batalhas, se um prazo estiver chegando, não faça nada. Na próxima vez que alguém revisar ou manter o código e continuar tendo problemas com ele, procure-os com a idéia de que, como equipe, você deve se concentrar mais na qualidade do código ao revisar o código, para que não tenha muitos problemas mais tarde.

Eles devem ver o valor antes de fazer o trabalho extra.

Eu sempre começo meu comentário com "gostaria", o que indica que o meu é simplesmente uma das muitas visualizações.

Eu também sempre incluo um motivo.

" Eu extrairia esse bloco em um método por causa da legibilidade."

Eu comento em tudo; grande e pequeno. Às vezes, faço mais de uma centena de comentários sobre uma mudança, caso em que também recomendo a programação em pares e me ofereço como ala.

Estou tentando estabelecer uma linguagem comum por razões; legibilidade, SECO, SRP, etc.

Também criei uma apresentação sobre Código Limpo e Refatoração, explicando o porquê e mostrando como, o que fiz para meus colegas. Eu o segurei três vezes até agora, e uma consultoria que estamos usando me pediu para segurá-lo novamente para eles.

Mas algumas pessoas não vão ouvir de qualquer maneira. Então eu fico com a posição de puxar. Eu sou o líder de design. A qualidade do código é minha responsabilidade. Essa alteração não será transmitida no meu relógio em seu estado atual.

Observe que estou mais do que disposto a recuar em qualquer comentário que fizer; por razões técnicas, prazos, protótipos etc. Ainda tenho muito o que aprender sobre codificação e sempre ouvirei a razão.

Ah, e recentemente me ofereci para comprar o almoço para o primeiro da minha equipe que enviou uma alteração não trivial sobre a qual eu não tinha nenhum comentário. (Ei, você tem que se divertir também. :-)

Às vezes, me encontro em uma situação em que o código de um parceiro é repleto de design confuso e pouca preocupação com manutenção futura, embora seja funcional e contenha pouco ou nenhum erro.

Este código está pronto. Em um determinado momento, os reprojetos se tornam muito caros para justificar. Se o código já estiver funcionando com pouco ou nenhum erro, será uma venda impossível. Sugira algumas maneiras de limpar isso no futuro e seguir em frente. Se / quando o código quebrar no futuro, reavalie o valor de um novo design. Pode nunca quebrar, o que seria ótimo. De qualquer maneira, você está no ponto em que faz sentido apostar que não vai quebrar, já que o custo será o mesmo agora ou mais tarde: redesenho prolongado e terrível.

O que você precisa fazer no futuro é ter iterações de desenvolvimento mais restritas. Se você tivesse conseguido revisar esse código antes de todo o trabalho de solucionar bugs ter sido investido, faria sentido sugerir um novo design. No final, nunca faz sentido fazer refatoração importante, a menos que o código seja escrito de uma maneira fundamentalmente insustentável e você tenha certeza de que o código precisará ser alterado logo após o lançamento.

Dada a escolha entre as duas opções (refatorar ou não refatorar), pense em quais parece a venda mais inteligente:

Ei, chefe, estávamos dentro do cronograma e tínhamos tudo funcionando, mas agora vamos reconstruir muitas coisas para podermos adicionar o recurso X no futuro.

ou

Ei chefe, estamos prontos para lançar. Se precisarmos adicionar o recurso X, pode levar alguns dias a mais.

Se você dissesse qualquer um, seu chefe provavelmente diria:

Quem disse algo sobre o recurso X?

O ponto principal é que, às vezes, um pouco de dívida técnica faz sentido, se você não pudesse corrigir certas falhas quando era barato (iterações iniciais). Ter um design de código de qualidade tem retornos decrescentes à medida que você se aproxima de um recurso concluído e do prazo.

[Essa resposta é um pouco mais ampla do que a pergunta originalmente colocada, pois esse é o redirecionamento para muitas outras perguntas sobre revisões de código.]

Aqui estão alguns princípios que considero úteis:

Critique em particular, elogie publicamente. Informe alguém sobre um bug no código individualmente. Se eles fizeram algo brilhante ou assumiram uma tarefa que ninguém queria, elogie-os em uma reunião de grupo ou em um e-mail enviado para a equipe.

Compartilhe seus próprios erros. Compartilho a história da minha desastrosa primeira revisão de código (recebida) com estudantes e colegas mais novos. Eu também deixei os alunos saberem que eu peguei o bug deles tão rapidamente, porque o fiz antes de mim. Em uma revisão de código, isso pode aparecer como: "Acho que você errou as variáveis ​​de índice aqui. Sempre verifico que, por causa do tempo, errei meus índices e reduzi o data center". [Sim, esta é uma história verdadeira.]

Lembre-se de fazer comentários positivos. Um breve "bom!" ou "truque legal!" pode tornar o dia de um programador júnior ou inseguro.

Suponha que a outra pessoa seja inteligente, mas às vezes descuidada. Não diga: "Como você espera que o chamador obtenha o valor de retorno se você realmente não o retorna ?!" Diga: "Parece que você esqueceu a declaração de retorno". Lembre-se de que você escreveu um código terrível nos seus primeiros dias. Como alguém disse uma vez: "Se você não tem vergonha do seu código de um ano atrás, não está aprendendo".

Salve o sarcasmo / ridículo para os amigos que não estão no seu local de trabalho. Se o código é épicamente horrível, brinque com ele em outro lugar. (Acho conveniente me casar com um colega programador.) Por exemplo, eu não compartilhava os seguintes desenhos (ou este ) com meus colegas.

Quando uma colher de açúcar ajuda o remédio a cair, e o que há de errado pode ser expresso de forma sucinta - não há 20 coisas erradas - vou liderar com uma forma que sugere que não tenho interesse, nem ego investido no que quero ser ouvido. Geralmente é algo como:

Gostaria de saber se seria melhor ...

ou

Faz algum sentido ...

Se os motivos são bastante óbvios, não os declaro. Isso dá a outras pessoas a chance de assumir alguma propriedade intelectual da sugestão, como em:

"Sim, é uma boa ideia, porque < sua razão óbvia aqui >."

Se a melhoria é bastante óbvia, mas não tanto, que me faça parecer um idiota por não pensar nela, e a razão para fazê-la reflete um valor compartilhado com o ouvinte, então às vezes nem o sugiro:

Gostaria de saber se existe uma maneira de ... <declaração de valor compartilhado aqui>

Isso é apenas para lidar com pessoas realmente delicadas - com a maioria dos meus colegas, eu apenas deixo eles entenderem!

As revisões de código nem sempre visam fazer melhorias.

Uma revisão próxima ao final de um projeto, como parece ser o caso aqui, é apenas para que todos saibam por onde começar a procurar quando os bugs aparecerem (ou para um projeto melhor projetado, o que pode estar disponível para reutilização posterior). Qualquer que seja o resultado da revisão, simplesmente não há tempo para mudar nada.

Para realmente fazer alterações, você precisa discutir o código e o design muito mais cedo no projeto - o código é muito mais fácil de mudar quando existe apenas como uma discussão sobre possíveis abordagens.

Sua pergunta é "Como revisar código mal projetado?":

A resposta IMO é simples. Fale sobre o DESIGN do código e como o design é defeituoso ou não atende aos requisitos. Se você apontar um design defeituoso ou "não atender aos requisitos", o desenvolvedor será forçado a alterar seu código, porque ele não faz o que precisa.

Se o código for "funcionalmente suficiente" e / ou "atender às especificações" e / ou "atender aos requisitos":

Se você é um colaborador deste desenvolvedor, não possui poder direto que permita "dizer a ele" para fazer alterações.

Existem algumas opções para você:

1. Você deve usar sua própria "influência" pessoal (uma forma de "poder" indireto) e / ou sua capacidade de ser "persuasivo"
2. envolva-se com o grupo "processo de código" de suas organizações e comece a tornar a "manutenção de código" uma prioridade mais alta.
3. Morda a bala e aprenda a ler códigos de baixa qualidade com mais rapidez / fluência, para que você não desligue (parece que você fica desligado ou fica mais lento quando encontra código de baixa qualidade) no código de baixa qualidade.
   • Isso também fará de você um programador mais forte.
   • E isso permitirá que você corrija o código de baixa qualidade quando estiver trabalhando no código de baixa qualidade.
   • E isso também permitirá que você trabalhe em mais projetos, porque muitos projetos têm códigos ruins que são funcionais ... mas muitos códigos ruins.
4. Lidere pelo exemplo. Faça seu código melhor ... mas não tente ser um perfeccionista.
   • Porque então você se tornará "o cara lento que não consegue cumprir os prazos, está sempre criticando e pensa que é melhor que todos os outros".

Acho que não há bala de prata. Você precisa usar todos os três e deve ser criativo no uso dos três.

No caso de um design prejudicial, seu foco deve ser maximizar o encapsulamento . Dessa forma, fica mais fácil substituir as classes / arquivos / sub-rotinas individuais por classes melhor projetadas.

Concentre-se em garantir que as interfaces públicas dos componentes sejam bem projetadas e que o funcionamento interno seja cuidadosamente oculto. Além disso, os wrappers de armazenamento de dados são essenciais. (Pode ser muito difícil alterar grandes quantidades de dados armazenados; portanto, se você "sangrar na implementação" em outras áreas do sistema, estará com problemas).

Depois de ter levantado as barreiras entre os componentes, concentre-se nos componentes com maior probabilidade de causar problemas importantes.

Repita até o prazo final ou até o sistema estar "perfeito".

Em vez de críticas diretas diretas ao código de alguém, é sempre melhor ser produtivo em nossos comentários durante a revisão do código.

Uma maneira que eu sigo é

1. será ótimo se fizermos dessa maneira.
2. Escrevê-lo dessa maneira fará com que seja executado mais rápido.
3. Seu código ficará muito mais legível se você fizer "isto" "isto" e "isto"

Tais comentários serão levados a sério, mesmo que seus prazos estejam chegando. E provavelmente será implementado no próximo ciclo de desenvolvimento.

A revisão de código deve ser integrada ao ciclo de cultura e desenvolvimento para funcionar. Não é provável que o agendamento de uma grande revisão de código no final do desenvolvimento do recurso X funcione. Primeiro de tudo, fazer as alterações será mais difícil e é provável que alguém se sinta envergonhado - criando resistência à atividade.

Você deve ter confirmações precoces e frequentes, juntamente com revisões no nível de confirmação. Com as ferramentas de análise de código em vigor, a maioria das revisões será rápida. Ferramentas de análise / revisão de código automatizadas, como FindBugs e PMD , ajudarão você a obter uma grande classe de erros de projeto imediatamente . No entanto, eles não o ajudarão a descobrir problemas de nível arquitetural, portanto, você deve ter um design sólido e julgar o sistema como um todo.

Aumente a qualidade das revisões de código.

Além da qualidade do código em revisão, existe uma qualidade da própria revisão de código:

• As mudanças propostas são realmente uma melhoria em relação ao que está presente?
• Ou apenas uma questão de opinião?
• A menos que algo muito óbvio, o revisor tenha explicado corretamente, por quê ?
• Quanto tempo levou? (Vi revisões com duração de meses, com o desenvolvedor sendo responsável pela resolução de todos os numerosos conflitos de mesclagem).
• Tom?

É muito mais fácil aceitar uma revisão de código de boa qualidade do que alguns cuidados com o ego, principalmente questionáveis.

Há duas questões notáveis ​​na questão, a parte com tato e o prazo que se aproxima . Essas são questões distintas - a primeira é uma questão de comunicação e dinâmica de equipe, a segunda é uma questão de planejamento e priorização.

Com muito tato . Suponho que você queira evitar egos escovados e respostas negativas contra os comentários. Algumas sugestões:

• Tenha um entendimento compartilhado dos padrões de codificação e dos princípios de design.
• Nunca critique ou revise o desenvolvedor , apenas o código . Evite a palavra "você" ou "seu código", apenas fale sobre o código em revisão, desanexado de qualquer desenvolvedor.
• Coloque seu orgulho na qualidade do código completo , de modo que os comentários de revisão que ajudam a melhorar o resultado final sejam apreciados.
• Sugira melhorias em vez de demanda. Sempre tenha um diálogo se não concordar. Tente entender o outro ponto de vista quando discordar.
• Faça com que os comentários sigam os dois lados. Ou seja, a pessoa que você revisou revisou seu código a seguir. Não tem críticas "unidirecionais".

A segunda parte é a priorização . Você tem muitas sugestões de melhorias, mas como o prazo está chegando, há apenas tempo para aplicar algumas.

Bem, primeiro você quer evitar que isso aconteça em primeiro lugar! Você faz isso executando revisões contínuas e incrementais. Não deixe um desenvolvedor trabalhar por semanas em um recurso e revise tudo no último momento. Em segundo lugar, as revisões de código e o tempo para implementar as sugestões de revisão devem fazer parte do planejamento e das estimativas regulares de qualquer tarefa. Se não houver tempo suficiente para revisar corretamente, algo deu errado no planejamento.

Mas vamos supor que algo deu errado no processo, e agora você se depara com vários comentários de revisão e não tem tempo para implementá-los. Você tem que priorizar. Em seguida, vá para as alterações que serão mais difíceis e arriscadas de serem alteradas posteriormente, se você a adiar.

A nomeação de identificadores no código-fonte é incrivelmente importante para facilitar a leitura e a manutenção, mas também é muito fácil e de baixo risco mudar no futuro. Mesmo com formatação de código. Portanto, não se concentre nessas coisas. Por outro lado, a sanidade das interfaces expostas ao público deve ser a maior prioridade, pois são realmente difíceis de mudar no futuro. É difícil alterar dados persistentes - se você começar a armazenar dados inconsistentes ou incompletos em um banco de dados, é realmente difícil de corrigir no futuro.

As áreas cobertas por testes de unidade são de baixo risco. Você sempre pode corrigi-los mais tarde. Áreas que não são, mas que podem ser testadas em unidade, apresentam menor risco do que áreas que não podem ser testadas em unidade.

Digamos que você tenha um grande pedaço de código sem testes de unidade e todos os tipos de problemas de qualidade de código, incluindo uma dependência codificada em um serviço externo. Ao injetar essa dependência, você torna o pedaço de código testável. Isso significa que você pode adicionar testes no futuro e, em seguida, trabalhar na correção do restante dos problemas. Com a dependência codificada, você não pode nem adicionar testes. Então, vá para essa correção primeiro.

Mas, por favor, tente evitar acabar neste cenário em primeiro lugar!