Acredito muito na regra dos escoteiros :
Sempre verifique um módulo mais limpo do que quando você fez o check-out. "Não importa quem seja o autor original, e se sempre fizermos algum esforço, por menor que seja, para melhorar o módulo. Qual seria o resultado? Acho que se todos seguissem essa regra simples, veríamos o fim da implacável deterioração de nossos sistemas de software.Em vez disso, nossos sistemas gradualmente se tornariam cada vez melhores à medida que evoluíam.Também veríamos equipes cuidando do sistema como um todo. do que apenas indivíduos cuidando de sua própria pequena parte.
Também acredito muito na ideia relacionada de Refatoração Oportunista :
Embora existam lugares para alguns esforços agendados de refatoração, prefiro incentivar a refatoração como uma atividade oportunista, realizada sempre e onde o código precisar ser limpo - por quem quer que seja. O que isso significa é que, a qualquer momento, alguém vê algum código que não é tão claro quanto deveria ser, deve aproveitar a oportunidade para corrigi-lo ali mesmo - ou pelo menos dentro de alguns minutos
Observe particularmente o seguinte trecho do artigo de refatoração:
Desconfio de qualquer prática de desenvolvimento que cause atrito para a refatoração oportunista ... Meu senso é que a maioria das equipes não faz refatoração suficiente, por isso é importante prestar atenção a qualquer coisa que esteja desencorajando as pessoas a fazê-lo. Para ajudar a esclarecer isso, fique atento a qualquer momento em que você se sentir desencorajado de fazer uma pequena refatoração, uma que, com certeza, levará apenas um ou dois minutos. Qualquer barreira desse tipo é um cheiro que deve desencadear uma conversa. Portanto, anote o desânimo e traga à equipe. No mínimo, isso deve ser discutido durante sua próxima retrospectiva.
Onde trabalho, há uma prática de desenvolvimento que causa atritos pesados - Revisão de Código (CR). Sempre que mudo algo que não esteja no escopo da minha "tarefa", sou criticado pelos meus revisores por dificultar a revisão. Isso é especialmente verdadeiro quando a refatoração está envolvida, pois dificulta a comparação de diferenças "linha a linha". Essa abordagem é o padrão aqui, o que significa que a refatoração oportunista raramente é feita, e apenas a refatoração "planejada" (que geralmente é muito pouco, muito tarde) ocorre, se é que existe.
Eu afirmo que os benefícios valem a pena e que três revisores trabalharão um pouco mais (para realmente entender o código antes e depois, em vez de olhar para o escopo restrito de quais linhas foram alteradas - a própria revisão seria melhor apenas por causa disso. ) para que os próximos 100 desenvolvedores que leem e mantenham o código se beneficiem. Quando apresento esse argumento, meus revisores dizem que não têm problemas com minha refatoração, desde que não esteja no mesmo CR. No entanto, afirmo que isso é um mito:
(1) Na maioria das vezes, você só percebe o que e como deseja refatorar quando está no meio de sua tarefa. Como Martin Fowler coloca:
Ao adicionar a funcionalidade, você percebe que algum código que está adicionando contém alguma duplicação com o código existente, portanto, é necessário refatorar o código existente para limpar as coisas ... Você pode conseguir algo funcionando, mas percebe que seria melhor se a interação com as classes existentes tiver sido alterada. Aproveite a oportunidade para fazer isso antes de se considerar terminado.
(2) Ninguém vai olhar favoravelmente para você liberando CRs "refatorando" que você não deveria fazer. Um CR tem uma certa sobrecarga e seu gerente não quer que você "perca seu tempo" em refatoração. Quando está incluído na alteração que você deve fazer, esse problema é minimizado.
O problema é exacerbado pelo Resharper, pois cada novo arquivo que adiciono à alteração (e não posso saber com antecedência exatamente quais arquivos acabariam sendo alterados) geralmente está repleto de erros e sugestões - a maioria dos quais está no local e merece totalmente fixação.
O resultado final é que eu vejo código horrível e deixo lá. Ironicamente, sinto que a fixação desse código não apenas não melhorará minha classificação, mas na verdade os reduzirá e me pintará como o cara "sem foco" que perde tempo consertando coisas que ninguém se importa em vez de fazer seu trabalho. Eu me sinto mal por isso, porque realmente desprezo o código incorreto e não suporto vê-lo, muito menos chamá-lo de meus métodos!
Alguma idéia de como posso remediar esta situação?
fonte
your manager doesn't want you to "waste your time" on refactoring
Respostas:
OK, então agora há mais coisas do que são adequadas para um comentário.
tl; dr
Sua intuição sobre o que você deve fazer (refatorar à medida que avança) está correta.
Sua dificuldade em implementar isso - considerando que você precisa solucionar um sistema de revisão de código ruim - se resume a sua dificuldade em manipular seu código fonte e o VCS. Várias pessoas disseram que você pode e deve dividir suas alterações (sim, mesmo em arquivos) em várias confirmações, mas você parece ter dificuldade em acreditar nisso.
Você realmente pode fazer isso. Isso é realmente o que estamos sugerindo. Você realmente deve aprender a tirar o máximo proveito de suas ferramentas de edição, manipulação de fontes e controle de versão. Se você investe tempo em aprender a usá-los bem, isso facilita a vida.
Problemas de política de fluxo de trabalho / escritório
Eu faria essencialmente a mesma recomendação que o GlenH7 de criar dois commits - um com apenas refatorações e (demonstrável ou obviamente) sem alterações funcionais e outro com as alterações funcionais em questão.
Pode ser útil, se você encontrar muitos erros, escolher uma única categoria de erro para corrigir em um único CR. Então você tem um commit com um comentário como "código de deduplicação", "corrija erros do tipo X" ou qualquer outra coisa. Como isso faz um único tipo de alteração, presumivelmente em vários lugares, deve ser trivial revisar. Isso significa que você não pode corrigir todos os erros encontrados, mas pode tornar menos doloroso o contrabando.
Problemas VCS
Dividir as alterações feitas na sua fonte de trabalho em várias confirmações não deve ser um desafio. Você não disse o que está usando, mas os possíveis fluxos de trabalho são:
se você estiver usando git, você tem excelentes ferramentas para isso
git add -i
para teste interativo na linha de comandogit gui
e selecionar blocos e linhas individuais para montar (esta é uma das poucas ferramentas GUI relacionadas ao VCS que eu realmente prefiro à linha de comando, a outra sendo um bom editor de mesclagem de 3 vias)rebase -i
se você não estiver usando o git, seu VCS ainda pode ter ferramentas para esse tipo de coisa, mas não posso aconselhar sem saber qual sistema você usa
Você mencionou que está usando o TFS - que acredito ser compatível com o git desde o TFS2013. Pode valer a pena experimentar o uso de um clone git local do repositório para trabalhar. Se isso estiver desativado ou não funcionar, você ainda poderá importar a fonte para um repositório git local, trabalhar nele e usá-lo para exporte seus commits finais.
você pode fazer isso manualmente em qualquer VCS se tiver acesso a ferramentas básicas como
diff
epatch
. É mais doloroso, mas certamente possível. O fluxo de trabalho básico seria:diff
para criar um arquivo de patch (contexto ou unificado) com todas as alterações desde a última confirmaçãopatch
para reproduzir um dos arquivos de correção, confirmar essas alteraçõesAgora você tem dois commits, com suas alterações particionadas adequadamente.
Note que provavelmente existem ferramentas para facilitar esse gerenciamento de patches - simplesmente não as uso, pois o git faz isso por mim.
fonte
Vou assumir que as Solicitações de Mudança são grandes e formais na sua empresa. Caso contrário, faça as alterações (sempre que possível) em muitos commits pequenos (como você deveria).
Continuar fazendo o que está fazendo?
Quero dizer, todos os seus pensamentos e deduções estão totalmente corretos. Você deve consertar as coisas que vê. As pessoas não fazem a refatoração planejada o suficiente. E esse benefício para todo o grupo é mais importante do que incomodar alguns.
O que pode ajudar é ser menos combativo. As revisões de código não devem ser combativas "Por que você fez isso?", Elas devem ser colaborativas "Ei, pessoal, enquanto eu estava aqui, eu arrumei tudo isso!". Trabalhar (com seu líder / gerente, se possível) para mudar essa cultura é difícil, mas é vital para criar uma equipe de desenvolvimento com alto desempenho.
Você também pode trabalhar (com seu líder / gerente, se possível) para promover a importância dessas idéias com seus colegas. Faça isso perguntando "por que vocês não se importam com a qualidade?" em vez de perguntar "por que você está sempre fazendo essas coisas inúteis?".
fonte
Sinto muita simpatia por sua situação, mas poucas sugestões concretas. Se nada mais, talvez eu o convença que, por pior que seja a situação, poderia ser pior. SEMPRE pode ser pior. :-)
Primeiro, acho que você tem (pelo menos) dois problemas com sua cultura, não apenas um. Um problema é a abordagem da refatoração, mas as revisões de código parecem um problema distinto. Vou tentar separar meus pensamentos.
Revisões de código
Eu estava em um grupo que odiava revisões de código. O grupo foi formado pela fusão de dois grupos de partes separadas da empresa. Eu vim do grupo que fazia revisões de código há vários anos, com resultados geralmente bons. A maioria de nós acreditava que as revisões de código eram um bom uso de nosso tempo. Nós nos fundimos em um grupo maior, e o mais próximo que pudemos dizer, que "outro" grupo nunca tinha ouvido falar em revisões de código. Agora estávamos trabalhando na base de código "deles".
As coisas não estavam bem quando nos fundimos. Os novos recursos demoravam 6 a 12 meses, ano após ano. A lista de pendências de bugs era enorme, crescente e drenava a vida. A propriedade do código era forte, especialmente entre os "gurus" mais antigos. "Ramos de recursos" às vezes duravam anos e se estendiam a alguns lançamentos; às vezes NINGUÉM, mas um único desenvolvedor viu o código antes de atingir o ramo principal. Na verdade, o "ramo de recursos" é enganoso, pois sugere que o código estava em algum lugar do repositório. Mais frequentemente, era apenas no sistema individual do desenvolvedor.
A gerência concordou que precisávamos "fazer alguma coisa" antes que a qualidade se tornasse inaceitavelmente baixa. :-) A resposta deles foi Code Reviews. A Code Reviews tornou-se um item oficial "Tu Shalt", antes de cada check-in. A ferramenta que usamos foi o Painel de Revisão.
Como funcionou na cultura que descrevi? Melhor do que nada, mas foi doloroso e levou mais de um ano para atingir uma espécie de nível mínimo de conformidade. Algumas coisas que observamos:
Eu pensei que iria escrever alguns parágrafos sobre Revisões de Código, mas acontece que eu estava escrevendo principalmente sobre cultura. Eu acho que tudo se resume a isso: as revisões de código podem ser boas para um projeto, mas uma equipe obtém apenas os benefícios que eles merecem.
Refatoração
Meu grupo desprezou a refatoração ainda mais do que odiava as revisões de código? Claro! Por todas as razões óbvias que já foram mencionadas. Você está desperdiçando meu tempo com uma revisão de código nojenta e nem está adicionando um recurso ou corrigindo um bug!
Mas o código ainda precisava desesperadamente de refatoração. Como proceder?
fonte
Por que você não faz as duas coisas, mas com confirmações separadas?
Seus colegas têm razão. Uma revisão de código deve avaliar o código que foi alterado por outra pessoa. Você não deve tocar no código que está revendo para outra pessoa, pois isso influencia sua função como revisor.
Mas se você vir vários problemas flagrantes, há duas opções que você pode seguir.
Se a revisão do código não for boa, permita que a parte que você revisou seja confirmada e refatore o código em um segundo check-in.
Se a revisão do código tiver problemas que precisem de correção, solicite ao desenvolvedor que refatorar com base nas recomendações do resharper.
fonte
Pessoalmente, odeio a ideia de revisar o código pós-commit. A revisão do código deve ocorrer à medida que você faz as alterações. É claro que estou falando aqui de programação em pares. Quando você emparelha, recebe a revisão gratuitamente e uma revisão de código melhor. Agora, estou expressando minha opinião aqui, sei que outros compartilham isso, provavelmente existem estudos que comprovam isso.
Se você conseguir que seus revisores de código emparelhem com você, o elemento combativo da revisão de código deve evaporar. Quando você começa a fazer uma mudança que não é compreendida, a questão pode ser levantada no ponto da mudança, discutida e exploradas alternativas que podem levar a melhores refatorações. Você obterá uma revisão de código de maior qualidade, pois o par será capaz de entender o escopo mais amplo da alteração, e não se concentrará tanto nas alterações linha a linha, que é o que você obtém na comparação lado a lado do código.
É claro que essa não é uma resposta direta ao problema de refatorações estarem fora do escopo da mudança que está sendo trabalhada, mas eu esperaria que seus colegas entendessem melhor o raciocínio por trás das mudanças se eles existissem quando você fez a alteração.
Além disso, supondo que você esteja fazendo TDD ou alguma forma de refator verde vermelho, uma maneira de garantir que você obtenha o envolvimento de seus colegas é usar a técnica de emparelhamento de ping pong. Simplesmente explicado à medida que o driver é girado para cada etapa do ciclo RGR, ou seja, o par 1 grava um teste com falha, o par 2 o corrige, o par 1 refatores, o par 2 escreve um teste com falha ... e assim por diante.
fonte
Provavelmente, esse problema reflete um problema muito maior com a cultura da organização. As pessoas parecem mais interessadas em fazer "seu trabalho" do que em melhorar o produto, provavelmente esta empresa tem uma cultura de "culpa" em vez de uma cultura colaborativa e as pessoas parecem mais interessadas em se cobrir do que em ter uma visão completa do produto / empresa .
Na minha opinião, você está completamente certo, as pessoas que revisam seu código estão completamente erradas se elas reclamarem porque você "toca" coisas fora de "sua tarefa", tenta convencer essas pessoas, mas nunca se opõe a seus princípios, para mim isso é a qualidade mais importante de um verdadeiro profissional.
E se a coisa certa dá a você números ruins de alguma maneira estúpida da corporação para avaliar seu trabalho, qual é o problema ?, quem quer "vencer" esse jogo de avaliação em uma empresa insana ?, tente mudá-lo! você está cansado, encontra outro lugar, mas nunca, nunca seja contra seus princípios, é o melhor que você tem.
fonte
Às vezes, refatorar é uma coisa ruim. Não pelas razões que seus revisores de código estão dando; parece que eles realmente não se preocupam com a qualidade do código, e você fazê- cuidado, que é impressionante. Mas há dois motivos que devem impedir você de refatorar : 1. Você não pode testar as alterações feitas com testes automatizados (testes de unidade ou o que for) ou 2. Você está fazendo alterações em algum código que não entende muito bem; ou seja, você não possui o conhecimento específico do domínio para saber quais alterações deve fazer.
1. Não refatorar quando não puder testar as alterações feitas com testes automatizados.
A pessoa que está revisando seu código precisa ter certeza de que as alterações feitas não quebraram nada do que costumava funcionar. Esse é o maior motivo para deixar o código ativo em paz. Sim, definitivamente seria melhor refatorar essa função de 1000 linhas de comprimento (que realmente é uma abominação!) Em várias funções, mas se você não pode automatizar seus testes, pode ser muito difícil convencer os outros de que você fez tudo direito. Eu definitivamente cometi esse erro antes.
Antes de refatorar, verifique se há testes de unidade. Se não houver testes de unidade, escreva alguns! Depois refatorar, e seus revisores de código não terão desculpa (legítima) por ficarem chateados.
Não refatorar trechos de código que exijam conhecimento específico do domínio que você não possui.
O local em que trabalho tem muito código pesado de engenharia química. A base de código usa os idiomas comuns aos engenheiros químicos. Nunca faça alterações idiomáticas em um campo. Pode parecer uma ótima idéia para mudar o nome de uma variável chamada
x
paramolarConcentration
, mas adivinhem? Em todos os textos de química, a concentração molar é representada com umx
. A renomeação torna mais difícil saber o que realmente está acontecendo no código para as pessoas desse campo.Em vez de renomear variáveis idiomáticas, basta colocar comentários explicando o que são. Se eles são não idiomática, por favor, renomeá-los. Não deixe a
i
,j
,k
,x
,y
, etc. flutuante em torno de quando nomes melhores funcionará.Regra geral para abreviações: se, quando as pessoas estão falando, elas usam uma abreviação, não há problema em usar essa abreviação no código. Exemplos da base de código no trabalho:
Temos os seguintes conceitos que sempre abreviamos quando falamos sobre eles: "Área de preocupação" se torna "AOC", "Explosão de nuvem de vapor" se torna VCE, coisas assim. Em nossa base de código, alguém refatorou todas as instâncias chamadas aoc para AreaOfConcern, VCE para vaporCloudExplosion, nPlanes para confiningPlanesCount ... o que tornou o código realmente muito mais difícil de ler para pessoas que tinham conhecimento específico do domínio. Também fui culpado de fazer coisas assim.
Na verdade, isso pode não se aplicar à sua situação, mas existem meus pensamentos sobre o assunto.
fonte
Agora, essa pergunta tem dois problemas distintos: facilidade de revisar alterações nas revisões de código e tempo gasto na refatoração.
Como outras respostas disseram - você pode separar os checkins de refatoração dos check-ins de alteração de código (não necessariamente como revisões separadas)? Dependendo da ferramenta que você está usando para fazer a revisão do código, você poderá visualizar as diferenças entre revisões específicas apenas (o Atlassian's Crucible definitivamente faz isso).
Se a refatoração for simples e facilitar a compreensão do código (que é tudo o que você deve fazer se for apenas refatoração), a revisão do código não deve demorar muito para ser concluída e deve ser uma sobrecarga mínima que é recuperada quando alguém precisa vir e olhar o código novamente no futuro. Se seus chefes não conseguem entender isso, talvez seja necessário incentivá-los gentilmente a alguns recursos que discutem por que a regra dos escoteiros leva a um relacionamento mais produtivo com seu código. Se você puder convencê-los de que o "desperdício de seu tempo" agora economizará duas, cinco ou dez vezes mais tarde, quando você / outra pessoa voltar para fazer mais trabalho nesse código, seu problema deverá desaparecer.
Você já tentou levar essas questões à atenção de seus colegas e discutir por que elas valem a pena ser corrigidas? E algum deles pode ser corrigido automaticamente (supondo que você tenha cobertura de teste suficiente para confirmar que não quebrou as coisas com a refatoração automática)? Às vezes, não vale a pena fazer uma refatoração, se é algo realmente exigente. Sua equipe inteira usa o ReSharper? Em caso afirmativo, você possui uma política compartilhada sobre quais avisos / regras estão sendo aplicados? Caso contrário, talvez você deva demonstrar onde a ferramenta está ajudando a identificar áreas do código que são possíveis fontes de sofrimento futuro.
fonte
Lembro-me de 25 anos atrás, "limpando" o código no qual eu estava trabalhando para outros fins, principalmente reformatando blocos de comentários e alinhamentos de guias / colunas para tornar o código puro e, portanto, mais fácil de entender (sem alterações funcionais reais) . Acontece que eu gosto de código limpo e bem ordenado. Bem, os desenvolvedores seniores ficaram furiosos. Acontece que eles usaram algum tipo de comparação de arquivo (diff) para ver o que havia mudado, em comparação com suas cópias pessoais do arquivo, e agora estava dando todos os tipos de falsos positivos. Devo mencionar que nossa biblioteca de códigos estava em um mainframe e não tinha controle de versão - você basicamente substituiu o que estava lá, e foi isso.
A moral da história? Não sei. Acho que às vezes você não pode agradar a ninguém, exceto a si mesmo. Eu não estava perdendo tempo (aos meus olhos) - o código limpo era mais fácil de ler e entender. É que as ferramentas primitivas de controle de alterações usadas por outras pessoas colocam algum trabalho extra de uma só vez. As ferramentas eram muito primitivas para ignorar espaço / tabulação e comentários atualizados.
fonte
Se você pode dividir a alteração solicitada e o refatoramento não solicitado em (muitas) confirmações separadas, conforme observado por @Useless, @Telastyn e outros, é o melhor que você pode fazer. Você ainda estará trabalhando em um único CR, sem a sobrecarga administrativa de criar um "refatorador". Apenas mantenha seus commits pequenos e focados.
Em vez de dar alguns conselhos sobre como fazê-lo, prefiro apontar uma explicação muito maior (de fato, um livro) de um especialista. Dessa forma, você poderá aprender muito mais. Leia Trabalhando efetivamente com o código herdado (de Michael Feathers) . Este livro pode ensiná-lo a fazer a refatoração, intercalada com as mudanças funcionais.
Os tópicos abordados incluem:
Este livro também inclui um catálogo de vinte e quatro técnicas de quebra de dependência que o ajudam a trabalhar com elementos do programa isoladamente e a fazer alterações mais seguras.
fonte
Eu também acredito muito na regra dos escoteiros e na refatoração oportunista. O problema geralmente está em conseguir a participação da gerência. A refatoração vem com riscos e custos. O que a administração geralmente ignora é que o mesmo ocorre com a dívida técnica.
É da natureza humana. Estamos acostumados a lidar com problemas reais, sem tentar evitá-los. Somos reativos, não pró-ativos.
O software é intangível e, portanto, é difícil entender que, como um carro, ele precisa de manutenção. Nenhuma pessoa razoável compraria um carro e nunca o repararia. Aceitamos que essa negligência diminuiria sua longevidade e, a longo prazo, seria mais cara.
Apesar de muitos gerentes entenderem isso, eles são responsabilizados por alterações no código de produção. Existem políticas que tornam mais fácil deixar bem o suficiente. Muitas vezes, a pessoa que precisa convencer está realmente acima do seu gerente e simplesmente não quer lutar para colocar suas "grandes idéias" em produção.
Para ser sincero, seu gerente nem sempre está convencido de que seu remédio é, de fato, tão bom quanto você pensa. (Óleo de cobra?) Há vendas. É seu trabalho ajudá-lo a ver o benefício.
A gerência não compartilha suas prioridades. Coloque seu chapéu de gerenciamento e veja com os olhos dele. Você provavelmente encontrará o ângulo certo. Seja persistente. Você efetuará mais alterações ao não permitir que a primeira resposta negativa o impeça.
fonte
Se você pode dividir a mudança solicitada e o refatoramento não solicitado em duas solicitações de mudança separadas, conforme observado por @ GlenH7, é o melhor que você pode fazer. Você não é "o cara que perde tempo", mas "o cara que trabalha duas vezes mais".
Se você se encontra frequentemente na situação em que não pode dividi-los, porque o trabalho solicitado agora precisa do refator não solicitado para compilar, sugiro que você insista em ter ferramentas para verificar automaticamente os padrões de codificação, usando os argumentos descritos aqui (o Resharper os avisos por si só podem ser um bom candidato, não estou familiarizado com isso). Esses argumentos são todos válidos, mas há um extra que você pode usar para sua vantagem: ter esses testes agora faz com que seja seu dever passar nos testes em cada confirmação.
Se sua empresa não deseja "perder tempo refatorando" (um sinal ruim da parte deles), deve fornecer um arquivo de "supressão de aviso" (cada ferramenta possui seu próprio mecanismo) para que você não fique mais irritado com tais avisos. Digo isso para fornecer opções para diferentes cenários, até no pior dos casos. Também é melhor ter acordos claramente definidos entre você e sua empresa, também sobre a qualidade esperada do código, em vez de suposições implícitas de cada lado.
fonte