Como posso sugerir com tato melhorias para o código mal projetado de outros durante a revisão?

1. Verifique novamente sua motivação. Se você acha que o código deve ser alterado, você deve ser capaz de articular alguma razão porque você acha que deve ser alterado. E essa razão deveria ser mais concreta do que "eu teria feito diferente" ou "é feio". Se você não pode apontar para algum benefício que vem de sua mudança proposta, então não há muito sentido em gastar tempo (dinheiro também) em mudá-lo.

2. Cada linha de código no projeto é uma linha que deve ser mantida. O código deve ser 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, é bom. Se você puder fazer isso enquanto aumenta a clareza, isso é muito melhor.

3. O código é como o concreto: é mais difícil mudar depois de ficar parado por um tempo. Sugira suas alterações mais cedo, se puder, para que o custo e o risco de alterações sejam minimizados.

4. Toda mudança custa dinheiro. Reescrever o código que funciona e provavelmente não precisa 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, há uma strong probabilidade de que ele também contenha erros. Procure esses erros e critique a funcionalidade defeituosa, em vez do recurso estético do código. Sugira melhorias que tornem o código mais eficiente e facilite a verificação do código.

6. Diferencie design e implementação. Uma classe importante com uma interface de baixa qualidade pode se espalhar através de 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 o dano. Por outro lado, uma classe com uma interface bem projetada, mas com uma implementação ruim, não deve ser um grande problema. Você sempre pode reimplementar a classe para melhor desempenho ou confiabilidade. Ou, se funcionar corretamente e for rápido o suficiente, você pode deixá-lo em paz e se sentir seguro sabendo que seu lixo está bem encapsulado.

Para resumir todos os pontos acima: Certifique-se de que as alterações propostas adicionam valor.

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

• melhore o código que provavelmente será alterado
• aumenta a clareza
• custa menos esforço

Considerações:

1. Sabemos que o código limpo é menos caro 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 ganhar, você só pode perder menos.
3. Concentre-se em adicionar valor real - não apenas beleza. Eu gosto do meu código para parecer bonito, mas às vezes tenho que aceitar mais questões baratas.
4. Uma boa maneira de encontrar o ponto ideal é seguir o Princípio do Escoteiro - quando você trabalha em uma área de código, sempre deixe-o em melhor forma do que encontrou.
5. Uma pequena melhoria é melhor do que nenhuma melhoria.
6. Faça bom uso de ferramentas automatizadas. Por exemplo, ferramentas que apenas limpam um pouco de formatação podem fazer um mundo de diferença.
7. Venda outras ideias que melhorem acidentalmente a clareza do código. Por exemplo, o teste de unidade incentiva a decomposição de grandes métodos em métodos menores.

Se o código funciona sem erros sérios, e um prazo maior (como nos efeitos P & L ou RP corporativa) é iminente, é tarde demais para sugerir melhorias que exigem grandes mudanças. Mesmo aprimoramentos no código podem criar riscos para a implantação do projeto. O tempo para melhorias foi no início do projeto, quando houve mais tempo para investir na robustez futura da base de código.

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

1. Verificando erros

2. Verificando onde o código pode ser melhorado

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

Avaliar a qualidade do design / código é, naturalmente, sobre # 2 e # 3.

Em relação a # 2:

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

  Por exemplo "A abordagem X ao design reduziria significativamente a probabilidade de ocorrência do bug Y ao fazer a mudança Z, e sabemos que esse código sofre alterações do tipo Z a cada duas semanas. O custo de lidar com a interrupção de produção do bug Y + encontrando o bug + consertar e liberar a correção + o custo de oportunidade de não entregar o próximo conjunto de recursos é $A ; considerando que o custo de limpar o código agora e o custo de oportunidade (por exemplo, preço do frete atrasado ou com menos recursos) é $B . , avalie - ou melhor, tenha seu líder / gerente de equipe - avalie $A vs $B e decida.

  • Isso ajudará a equipe inteligente a gerenciar isso de maneira eficaz. Por exemplo. eles tomarão uma decisão racional usando informações completas

  • Isso aumentará (especialmente se você expressar isso bem) para aumentar o status da sua conta - por exemplo, você é alguém inteligente o suficiente para ver os benefícios de um design melhor, E inteligente o suficiente para não exigir religiosamente, sem pesar considerações comerciais.

  • E, no caso provável de ocorrer o bug Z, você ganhará muito mais influência no próximo conjunto de sugestões.

Em relação a # 3:

• MUITO claramente delinear "deve corrigir" erros / problemas de "Esta é uma prática recomendada e realmente deve ser corrigida SE nós pudermos poupar recursos - veja a análise pro / con anexada" melhorias de design (adivinhe as coisas descritas para o item 2 acima ) "Estas são diretrizes gerais que, acredito, ajudariam a melhorar a robustez do seu código, para que você possa manter com mais facilidade o código" de alterações opcionais. Por favor, note o texto - não é sobre "fazer o seu código como o que eu quero" - é "se você fizer isso, você ganha os benefícios a, b, c". O tom e a abordagem são importantes.

Escolha suas batalhas, se um prazo final estiver chegando, não faça nada. A próxima vez que outra pessoa estiver revisando ou mantendo o código e continuar tendo problemas com ela, aproxime-se deles com a ideia de que, como equipe, você deve se concentrar mais na qualidade do código ao revisar o código para não ter tantos problemas posteriormente.

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

Eu sempre começo meu comentário com "eu gostaria", o que indica que o meu é simplesmente um dos muitos modos de exibição.

Eu também incluo sempre um motivo.

" gostaria de extrair este bloco em um método porque de legibilidade."

Comento tudo; grande e pequeno. Às vezes faço mais de uma centena de comentários sobre uma mudança, e nesse caso também recomendo a programação em pares e me ofereço como wing-man.

Estou tentando estabelecer uma linguagem comum por motivos; legibilidade, DRY, SRP, etc.

Eu também criei uma apresentação sobre Código Limpo e Refatoração explicando por que e mostrando como, o que eu mantive para meus colegas. Eu segurei isso três vezes até agora, e uma consultoria que estamos usando me pediu para fazer isso de novo para eles.

Mas algumas pessoas não escutam de qualquer maneira. Então eu fiquei 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á exibida no meu relógio no estado atual.

Por favor, note que eu estou mais do que disposto a desistir de qualquer comentário que eu faça; por motivos técnicos, prazos, protótipos, etc. Ainda tenho muito a aprender sobre codificação e sempre ouço 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 na qual eu não tinha qualquer comentário. (Ei, você tem que se divertir também.: -)

[Esta resposta é um pouco mais ampla do que a questão originalmente formulada, já que este é o redirecionamento para muitas outras perguntas sobre revisões de código.]

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

Crítica em particular, elogie publicamente. Informe alguém sobre um bug em seu 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 email enviado para a equipe.

Compartilhe seus próprios erros. Compartilho a história da minha desastrosa primeira análise de código (recebida) com alunos e colegas juniores. Eu também deixo os alunos saberem que eu peguei o bug deles tão rápido porque eu fiz isso antes de mim mesmo. Em uma revisão de código, isso pode sair como: "Acho que você errou as variáveis do índice aqui. Eu sempre verifico isso por causa da hora em que eu errei meus índices e derrubou um data center". [Sim, esta é uma história verdadeira.]

Lembre-se de fazer comentários positivos. Um breve "bom!" ou "truque puro!" 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 devolver?" Diga: "Parece que você esqueceu a declaração de retorno". Lembre-se de que você escreveu um código horrível em 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 amigos que não estão no seu local de trabalho. Se o código for épico, piada sobre isso em outro lugar. (Acho conveniente ser casada com um colega programador.) Por exemplo, eu não compartilharia os seguintes desenhos animados (ou este ) com meus colegas.

I sometimes find myself in a situation where a peer's code is riddled with messy design and very little concern for future maintenance, though it's functional and contains little to no bugs.

Este código está pronto. Em um certo ponto, redesenhos se tornam muito caros para justificar. Se o código já estiver funcionando com pouco ou nenhum bug, isso 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 forma, 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 horrível.

O que você precisa fazer no futuro é ter iterações de desenvolvimento mais estreitas. Se você tivesse sido capaz de rever este código antes que todo o trabalho de engordar bugs tivesse sido investido, faria sentido sugerir um novo design. No final, nunca faz sentido fazer grandes refatorações a menos que o código seja escrito de uma forma 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 sobre o que parece ser a venda mais inteligente:

Hey boss, we were on schedule and had everything working, but now we are going to rebuild a lot of stuff so that we can add feature X in the future.

ou

Hey boss, we are ready to release. If we ever have to add on feature X, it might take us a couple extra days.

Se você disse qualquer um deles, seu chefe provavelmente diria:

Who said anything about feature X?

Quando uma colherada de açúcar ajuda o remédio a diminuir, e o que está errado pode ser expresso de forma sucinta - não há 20 coisas erradas - eu liderei com um formulário que sugere que não tenho participação, nenhum ego investido o que eu quero ser ouvido. Geralmente é algo como:

I wonder if it would be better to...

ou

Does it make any sense to...

Se as razões são bastante óbvias, não as 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 como para me fazer parecer um idiota por não pensar nisso, e a razão para fazê-lo reflete um valor compartilhado com o ouvinte, então em algum momento eu nem sequer sugiro isso em vez disso:

Eu me pergunto se há uma maneira de ... < declaração de valor compartilhado aqui >

Isto é apenas para lidar com pessoas realmente sensíveis - com a maioria dos meus colegas, eu apenas deixo que eles o tenham!

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

Uma revisão perto do final de um projeto, como parece ser o caso aqui, é apenas para que todos saibam onde começar a procurar quando os bugs entrarem (ou para um projeto melhor projetado, o que pode estar disponível para reutilização posterior). Seja qual for o resultado da revisão, simplesmente não há tempo para mudar nada.

Para realmente fazer mudanças, você precisa discutir o código e projetar muito mais cedo no projeto - o código é muito mais fácil de alterar quando existe apenas como falar sobre possíveis abordagens.

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

A resposta da 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 ao requisito", o desenvolvedor será forçado a alterar seu código porque não faz o que precisa fazer.

Se o código for "funcionalmente suficiente" e / ou "atende à especificação" e / ou "atende aos requisitos":

Se você for um colaborador desse desenvolvedor, não terá nenhum 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" que é 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 priorizar a "manutenção de código".
3. Morda a bala e aprenda a ler código de baixa qualidade mais rápido / mais fluentemente para que você não fique preso (parece que você fica pendurado ou desacelerado quando encontra código de baixa qualidade) em código de baixa qualidade.
   • Isso também fará de você um programador mais strong.
   • E ele permitirá que você corrija o código quando estiver trabalhando em código de baixa qualidade.
   • E isso também permitirá que você trabalhe em mais projetos, pois muitos projetos têm código de baixa qualidade que é funcional ... mas muitos códigos de baixa qualidade.
4. Liderar pelo exemplo. Torne seu código melhor ... mas não tente ser um perfeccionista.
   • Porque então você vai se tornar "o cara lento que não pode cumprir prazos, está sempre criticando, e acha que ele é melhor do que todo mundo".

Eu acho que não há bala de prata. Você tem que usar todos os três e você tem que ser criativo no uso de todos os três.

No caso de um projeto extremamente ruim, seu foco deve ser maximizar o encapsulamento . Dessa forma, fica mais fácil substituir classes / arquivos / sub-rotinas individuais por classes mais bem projetadas.

Concentre-se em garantir que as interfaces públicas dos componentes sejam bem projetadas e que os trabalhos internos sejam cuidadosamente ocultados. Além disso, os wrappers de armazenamento de dados são essenciais. (Grandes quantidades de dados armazenados podem ser muito difíceis de alterar, por isso, se você tiver "sangramento de implementação" em outras áreas do sistema, estará com problemas).

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

Repita até o prazo final ou até que o sistema esteja "perfeito".

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

Uma maneira que eu sigo é

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

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 mudanças será mais difícil e é provável que alguém se sinta envergonhado - criando resistência à atividade.

Você deve ter commits iniciais e frequentes, juntamente com revisões no nível de commit. Com as ferramentas de análise de código em vigor, a maioria das análises será rápida. Análise automatizada de código / ferramentas de revisão, como FindBugs e PMD irá ajudá-lo a obter uma grande classe de erros de projeto fora da porta. No entanto, eles não ajudarão você a resolver problemas de nível arquitetônico, portanto, você deve ter um projeto sólido e julgar o sistema como um todo em relação a esse design.

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

Além da qualidade do código em análise, existe uma qualidade de revisão do código em si:

• As mudanças propostas são realmente um aprimoramento sobre o que está presente?
• Ou apenas uma questão de opinião?
• A menos que algo muito óbvio, o revisor tenha explicado corretamente, por que ?
• Quanto tempo demorou? (Eu tenho visto revisões com duração de meses, com o desenvolvedor sob revisão responsável por resolver todos os numerosos conflitos de mesclagem).
• Tom?

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

Existem dois problemas notáveis na questão, a parte tactfully e a parte do prazo final . Estas 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.

Tactfully . Eu suponho que você quer evitar egos escovados e pushback negativo contra os comentários. Algumas sugestões:

• Tenha uma compreensão compartilhada sobre os padrões de codificação e 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 análise, desanexado de qualquer desenvolvedor.
• Coloque seu orgulho na qualidade do código concluído , de modo que os comentários de revisão que ajudam a melhorar o resultado final sejam bem-vindos.
• Sugerir melhorias em vez de exigir. Sempre tenha um diálogo se você não concordar. Tente entender o outro ponto de vista quando você discordar.
• Faça as avaliações nos dois sentidos. Ou seja peça à pessoa que você revisou revise seu código a seguir. Não tem comentários "unidirecionais".

A segunda parte é a priorização . Você tem muitas sugestões de melhorias, mas desde que o prazo se aproxima, só há tempo para aplicar algumas.

Bem, primeiro você quer evitar que isso aconteça em primeiro lugar! Você faz isso realizando revisões contínuas e incrementais. Não deixe um desenvolvedor trabalhar por semanas em um recurso e, em seguida, 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 adequadamente, algo está errado no planejamento.

Mas vamos supor que algo deu errado no processo, e agora você está diante de vários comentários de revisão e não tem tempo para implementá-los todos. Você tem que priorizar. Então vá para as mudanças que serão mais difíceis e arriscadas para mudar depois, se você adiá-lo.

A nomeação de identificadores no código-fonte é extremamente importante para a legibilidade e a facilidade de manutenção, mas também é muito fácil e de baixo risco mudar no futuro. O mesmo com formatação de código. Então não se concentre nessas coisas. Por outro lado, a sanidade das interfaces expostas publicamente deve ser a prioridade mais alta, já que elas são realmente difíceis de mudar no futuro. Dados persistentes são difíceis de mudar - se você começar a armazenar dados inconsistentes ou incompletos em um banco de dados, será muito difícil consertá-los no futuro.

As áreas cobertas pelos testes unitários são de baixo risco. Você sempre pode consertar isso depois. As áreas que não são, mas que poderiam ser testadas em unidades são de menor risco do que as áreas que não podem ser testadas em unidades.

Digamos que você tenha uma grande quantidade 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 bloco de código testável. Isso significa que você pode adicionar testes e, em seguida, trabalhar na correção do restante dos problemas. Com a dependência codificada, você não pode adicionar testes. Então, vá para essa correção primeiro.

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