É útil mini-refatorar o código na esperança de melhorar a qualidade ou é apenas "mudar o código" sem muito benefício?

12

Exemplo

Me deparei com um código monolítico que faz "tudo" em um só lugar - carregando dados do banco de dados, mostrando a marcação HTML, agindo como um roteador / controlador / ação. Comecei a aplicar o código do banco de dados móvel SRP em seu próprio arquivo, fornecendo nomes melhores para as coisas, e tudo parecia bom, mas comecei a ter dúvidas sobre o motivo de estar fazendo isso.

Por que refatorar? Qual é o propósito? Isso é inútil? Qual o benefício? Observe que na maioria das vezes deixei o arquivo monolítico como está, mas refatorei apenas a parte menor que era relevante para a área em que eu precisava fazer algum trabalho.

Código original:

Para dar um exemplo concreto, deparei-me com esse trecho de código - ele carrega as especificações do produto por um ID de produto conhecido ou por um ID de versão selecionado pelo usuário:

if ($verid)
    $sql1 = "SELECT * FROM product_spec WHERE id = " . clean_input($verid);
else
    $sql1 = "SELECT * FROM product_spec WHERE product_id = " . clean_input($productid) ;
$result1 = query($sql1);
$row1 = fetch_array($result1);
/* html markup follows */

Reestruturação:

Como estou fazendo algum trabalho exigindo que eu mude as coisas nesta parte específica do código, mudei para usar o padrão de repositório e atualizei-o para usar os recursos MySQL orientados a objetos:

//some implementation details omitted 
$this->repository = new SpecRepository($mysql);
if ($verid)
    $row1 = $this->repository->getSpecByVersion($verid);
else 
    $row1 = $this->repository->getSpecByProductId($productid);
/* html markup follows to be refactored or left alone till another time*/

//added new class:
class SpecRepository extends MySqlRepository
{

    function getSpecByVersion(int $verid)
    {
        return $this->getMySql()->paramQuery("
            SELECT * FROM product_spec WHERE id = ?
        ", $verid)->getSingleArray();
    }

    function getSpecByProductId(int $productid)
    {
        return $this->getMySql()->paramQuery("
            SELECT * FROM product_spec WHERE product_id = ?
        ", $productid)->getSingleArray();
    }
}

Devo fazer isso?

Olhando para as mudanças, o código ainda está lá, com a mesma funcionalidade, mas em arquivos diferentes, nomes diferentes, lugares, usando mais estilo orientado a objetos do que procedimental. Na verdade, é engraçado notar que o código refatorado parece muito mais inchado, apesar de ter a mesma funcionalidade.

Prevejo algumas respostas dizendo "se você não sabe as razões pelas quais refatorar, não faça", e talvez eu concorde. Minha razão é melhorar a qualidade do código ao longo do tempo (e minha esperança é que eu o faça seguindo o SRP e outros princípios).

Essas são boas razões ou estou desperdiçando meu tempo em "reorganizar o código" dessa maneira? A refatoração geral parece um pouco como pisar na água para ser honesto - leva tempo e fica mais "separado" no que diz respeito ao SRP, mas, apesar das minhas boas intenções, não sinto que estou fazendo melhorias incríveis. Portanto, debatendo se é melhor deixar o código como antes e não refatorar.

Por que refatorei em primeiro lugar?

No meu caso, estou adicionando novas funcionalidades a uma nova linha de produtos; portanto, devo seguir a estrutura de código existente para linhas de produtos similares ou escrever a minha.

Dennis
fonte
OT, mas algo a considerar é que Select * from ..pode ser considerado um antipadrão. Veja stackoverflow.com/q/3639861/31326
Peter M
1
@ PeterM: A menos que você esteja escrevendo um ORM, nesse caso, select *torna-se uma "melhor prática".
Robert Harvey
@RobertHarvey No caso que estou questionando por que você está escrevendo seu próprio ORM: D
Peter M
Fiz refatores por causas mais irrelevantes. Reduzir a dívida técnica é bom assim que os benefícios superam os custos. Parece que este é o seu caso. A pergunta é: você tem os testes certos para garantir que o código ainda funcione conforme o esperado?
LAIV
1
A confiabilidade deve ser a métrica predominante ao refatorar a IMO, não a capacidade de manutenção relacionada à capacidade de mudança, uma vez que a confiabilidade reduz os motivos para que as coisas mudem em primeiro lugar. Ele melhora a estabilidade e o código que é perfeitamente estável e confiável e cumpre tudo o que é necessário gera um pequeno custo de manutenção, uma vez que há muito poucas razões para mudar. Procure reduzir as razões pelas quais as coisas mudam, em vez de tentar torná-las o mais fácil possível. Procurar o primeiro também tenderá a dar alguns dos últimos.

Respostas:

13

No exemplo concreto que você deu, a refatoração pode não ter sido necessária, pelo menos ainda não. Mas você viu que havia um potencial para códigos mais confusos no futuro, o que levaria a uma refatoração mais longa e mais tediosa, então você tomou a iniciativa e limpou as coisas agora.

Eu também diria que a vantagem que você ganhou aqui foi um código que é mais fácil de entender, pelo menos do meu ponto de vista, como alguém que não conhece sua base de código.


Em geral:

A refatoração facilita a compreensão de seu código por outros desenvolvedores?

A refatoração facilita o aprimoramento do código?

A refatoração facilita a manutenção do código?

A refatoração facilita a depuração?

Se a resposta para qualquer uma dessas perguntas fosse "sim", valeria a pena e seria mais do que apenas "mover código"?

Se a resposta para tudo isso foi "não", talvez não precise ser refatorada.

O tamanho final da base de código pode ser maior em termos de LoC (embora algumas refatorações possam reduzir a base de código simplificando o código redundante), mas isso não deve ser um fator se houver outros ganhos.

FrustratedWithFormsDesigner
fonte
9

A refatoração incha se você estiver desmontando um monólito.

No entanto, você já encontrou o benefício.

"No meu caso, estou adicionando novas funcionalidades para uma nova linha de produtos."

Você não pode adicionar funcionalidades a um monólito com muita facilidade sem quebrar as coisas.

Você disse que o mesmo código está obtendo os dados e fazendo a marcação. Ótimo, até você querer alterar quais colunas você está selecionando e manipulando para mostrar em determinadas condições. De repente, seu código de marcação para de funcionar em um determinado caso e você deve refatorar.

Diácono
fonte
sim, mas também posso continuar adicionando ao monólito mesmo para novas funcionalidades :) ou seja, eu adicionaria um novo if/elsebloco e seguiria o estilo original de código para adicionar instruções SQL e, em seguida, colocaria nova marcação HTML no mesmo arquivo de monólito. E para alterar as colunas, considero o bloco if / else responsável por hospedar as linhas SQL e edito esse SQL para fazer alterações nas colunas, sem quebrar esse arquivo.
Dennis
Com a abordagem refatorada, eu provavelmente iria primeiro ao bloco if / else no monólito para ver que preciso ir ao arquivo de repositório separado para alterar o SQL. Ou, se eu trabalhasse naquele monólito recentemente, saberia ir direto para o arquivo do repositório, ignorando o monólito. Ou se eu procurei a minha base de código para o SQL específica a busca iria me colocar para o novo arquivo de repositório também ignorando o monólito
Dennis
2

Considero refatorar se souber que terei que trabalhar meses ou até anos com esse projeto. Se eu tiver que fazer uma alteração aqui e ali, então resisto à vontade de mudar o código.

A maneira preferida de refatorar é quando eu tenho um código base com algum tipo de teste automatizado. Mesmo assim, tenho que ter muito cuidado com o que mudo para garantir que as coisas estejam funcionando como antes. Você perderá a confiança muito em breve se entregar erros em outras áreas além daquela que deveria trabalhar.

Valorizo ​​a refatoração pelos seguintes motivos:

  • Eu entendo o código melhor
  • Como removo o código duplicado, se houver um erro em um local, não preciso procurar em todos os lugares em que o código foi copiado e colado
  • Crio classes que têm seu papel bem definido. Por exemplo, posso reutilizar o mesmo repositório para hidratar uma página HTML, um feed XML ou um trabalho em segundo plano. Isso não seria possível se o código de acesso ao banco de dados residisse apenas na classe HTML
  • Renomeio variáveis, métodos, classes, módulos, pastas, se não corresponderem à realidade atual; I commento código através de nomes de variáveis, nomes de métodos
  • Resolvo bugs complexos com uma combinação entre testes de unidade e refatoração
  • Eu tenho a flexibilidade de substituir módulos inteiros, pacotes. Se o ORM atual estiver desatualizado, com erros ou sem manutenção, será mais fácil substituí-lo se não estiver espalhado por todo o projeto.
  • Descubro novos recursos ou aprimoramentos que resultam em propostas para o cliente.
  • Criei componentes reutilizáveis. Esse foi um dos maiores benefícios, porque o trabalho realizado nesse projeto poderia ter sido reutilizado para outro cliente / projeto.
  • Costumo começar com a solução mais idiota, codificada e maluca e refatá-la mais tarde, quando aprender mais. Este momento posterior pode ser hoje ou talvez depois de vários dias. Não quero gastar muito tempo com architecta solução. Isso ocorrerá durante a escrita - reescrevendo o código para frente e para trás.

Em um mundo perfeito, as refatorações devem ser verificadas por testes de unidade, testes de integração etc. Se não houver, tente adicioná-los. Além disso, alguns IDE podem ajudar bastante. Eu costumo usar um plugin que custa dinheiro. Quero passar pouco tempo com refatorações e ser muito eficiente.

Eu também introduzi bugs. Percebo o motivo pelo qual algum controle de qualidade está perguntando. are you guys doing refactoring? because everything stopped working!Esse é o risco que a equipe precisa abraçar e sempre precisamos ser transparentes.

Durante os anos, descobri que a refatoração contínua melhorava minha produtividade. Foi muito mais fácil adicionar novos recursos. Além disso, as grandes reconstruções não exigiam uma reescrita completa, como costumava acontecer quando a base de código não era adaptada à evolução do produto. Também é divertido.

Adrian Iftode
fonte
Isso está muito próximo de uma votação negativa - Não existe um 'eu' em 'Programador de computador'. Por 'eu' você me 'eu' (voto negativo) ou você quer dizer os desenvolvedores trabalhando no código agora e no futuro?
mattnz
Eu também notei isso, mas coloquei no texto. Também trabalhei como desenvolvedor solo por muitos anos e, quando escrevi essa resposta, reiterei principalmente dessa experiência. Eu poderia substituir Icom toembora.
Adrian Iftode
1

Eu acho que a pergunta que você deve se fazer não é tanto "Existe algum benefício?" mas "Quem pagará por este trabalho?"

Todos nós podemos discutir sobre os benefícios percebidos até o final dos dias, mas, a menos que você tenha um cliente que acredite nesses benefícios e os valorize o suficiente para pagar pelas mudanças, não deveria fazê-los.

É muito fácil quando você está na equipe de desenvolvimento em algum lugar para ver o código e deseja refatorá-lo para torná-lo 'melhor'. Mas colocar um valor em 'código melhor' quando o produto já funciona é realmente difícil.

Coisas como 'desenvolvimento mais rápido de novos recursos' não são suficientes porque são apenas custos reduzidos. Você precisa gerar vendas.

Ewan
fonte
1

Na minha opinião, refatorar é um bom investimento.

Caso contrário, em breve você terá dívidas técnicas (problemas não resolvidos, código incorreto que funciona apenas sob determinadas condições etc.). As dívidas técnicas em breve ficarão cada vez maiores, até que o software não seja mais sustentável.

Mas para fazer a refatoração funcionar, você também precisa investir em testes. Você deve criar testes automatizados; idealmente, você deve ter testes de unidade e testes de integração. Isso evita que você quebre o código existente.

Se você precisar convencer seu chefe ou colegas, leia alguns livros sobre métodos ágeis (por exemplo, SCRUM) e desenvolvimento orientado a testes.

Bernie
fonte