Você abre uma função para corrigir um bug de uma linha e quarenta minutos depois ainda está rastreando qual ramo define qual variável. Três tarefas sem relação entre si estão soldadas em um único bloco, e nenhuma delas tem nome. Você puxa um fio e não consegue saber quais outros se movem.
Isso é spaghetti code, e o nome não tem nada a ver com indentação ou estilo de nomenclatura. Ele descreve o fluxo de controle: a execução pula entre condições da mesma forma que um fio de macarrão cruza todos os outros na tigela, e seguir um único caminho até o fim significa manter quatro níveis de if na cabeça ao mesmo tempo.
O que realmente torna o código spaghetti code
Quatro sintomas aparecem juntos em quase todos os casos, e cada um pode ser corrigido isoladamente:
- Aninhamento profundo. Três ou quatro níveis de
if/elseum dentro do outro. Cada nível adiciona indentação e uma condição que você precisa manter verdadeira na cabeça para saber qual linha está lendo. - Funções que fazem vários trabalhos. Uma função valida a entrada, calcula um resultado, altera o estado e escreve um log. Você muda o cálculo e arrisca o log; testa a validação e precisa simular o estado.
- Estado compartilhado oculto. Uma variável global ou um dicionário no nível do módulo, alterado de dentro de uma função cujo nome não dá nenhuma pista disso. Duas chamadas podem pisar uma na outra e o código de nenhuma delas mostra por quê.
- Ramos copiados e colados. A mesma forma de lógica repetida com um valor alterado, em vez de extraída uma vez. Você corrige uma cópia, esquece a outra, e tem um bug que só aparece em metade das entradas.
Nenhum desses problemas exige uma reescrita. Cada um tem um refactoring com nome próprio que o elimina sem mudar o que o código faz. Essa restrição importa mais do que parece: se você notar um bug enquanto limpa o código, não corrija na mesma mudança. Faça isso separadamente, ou não vai conseguir saber qual mudança causou qual regressão.
Uma função real com todos os sintomas do spaghetti code
Aqui está process_order, do fluxo de checkout de uma loja pequena. Leia uma vez e note onde você perde o fio.
| |
Os quatro sintomas estão aqui. A linha que cobra o cliente está quatro níveis abaixo. A função verifica o estoque, calcula os preços, aplica um desconto de fidelidade, altera INVENTORY, envia (via print, no lugar de uma chamada de envio real) e totaliza o pedido. INVENTORY é um dicionário no nível do módulo que qualquer outra função também pode alterar, e nada na assinatura avisa sobre isso. Os ramos VIP e não VIP têm o mesmo formato com números diferentes, então um terceiro nível de desconto significa uma terceira cópia.
Como refatorar spaghetti code sem quebrá-lo
Uma mudança pequena e mecânica de cada vez. Cada uma mantém o comportamento idêntico e tem seu próprio commit. Se o passo 3 quebrar algo, o git bisect consegue encontrar qual dos cinco commits pequenos foi. Um único commit “refatorar tudo” não deixa nada para o bisect restringir.
Trave o comportamento com um teste de caracterização
Antes de mudar uma linha, escreva um teste que registre o que a função faz hoje. Não o que ela deveria fazer. Isso é um teste de caracterização: você está congelando o comportamento atual para perceber o momento em que uma mudança “segura” deixa de ser segura.
| |
Rode os dois primeiro contra a função ainda intocada, para saber que os números esperados vêm do código e não de um erro de digitação. Reinicie INVENTORY antes de cada teste. Cada passo abaixo roda contra esse mesmo par. Enquanto eles passarem, o comportamento não mudou.
Extraia funções para preço e frete
Extraia a lógica de preço e frete em funções cujo nome diga o que fazem. A lógica em si não muda nesse passo.
| |
Rode os testes: os mesmos dois números, a mesma contagem de estoque. Faça o commit com uma mensagem que diga exatamente o que aconteceu: “extract price_line_item and calculate_shipping, no behavior change”.
Substitua condicionais aninhados por guard clauses
As verificações de estoque em process_order somam três níveis de aninhamento em torno da única linha que importa. Transforme cada verificação em uma saída antecipada para o caso que não pode prosseguir, e o caminho de sucesso deixa de ficar indentado sob tudo o resto.
| |
Mesmos testes, mesmos resultados. O corpo do loop agora se lê de cima para baixo: rejeita, rejeita, faz o trabalho. São os mesmos ramos de antes, um nível de profundidade em vez de três.
Substitua o argumento flag por uma tabela de lookup
is_vip muda o que price_line_item faz internamente. Todo ponto de chamada precisa saber que o booleano existe, e um terceiro nível significa um terceiro ramo copiado e colado. Mova os números para dados:
| |
process_order muda da mesma forma: seu parâmetro é renomeado de is_vip para tier, e a única chamada a price_line_item(item, is_vip) vira price_line_item(item, tier). Nada mais no corpo da função muda. Uma chamada como process_order(order, tier="vip") se lê sozinha, e um novo nível é uma nova entrada em DISCOUNT_RATES em vez de mais um ramo if/else. Esse é o único passo que muda uma assinatura, então os testes precisam de um ajuste correspondente: passe tier="vip" em vez de is_vip=True. As asserções continuam como estão, porque a saída para o mesmo pedido não mudou.
Leve os efeitos colaterais para as bordas
price_line_item agora é pura: mesmas entradas, mesma saída, nada é alterado. process_order ainda altera INVENTORY e chama print no meio de um loop, por isso testá-la exige um reset global em vez de uma simples chamada de função. Separe decidir de fazer. Calcule o que deveria acontecer, retorne isso, e deixe quem chamou agir de acordo.
| |
Dois detalhes garantem que isso seja uma correspondência exata com o original, não apenas aproximada. O ramo do pedido vazio ainda passa por calculate_shipping em vez de retornar um 0 fixo: o original cobra frete mesmo em um pedido vazio, então o refactoring precisa continuar cobrando também. E plan_order verifica o estoque contra uma cópia local, remaining, que vai decrementando a cada item, não diretamente contra INVENTORY. Assim, um pedido com o mesmo SKU em duas linhas é verificado contra um estoque decrescente, exatamente como fazia o decremento inline do original. Verificar as duas linhas contra o mesmo número intocado deixaria uma segunda linha passar em uma verificação que a primeira já deveria ter esgotado.
plan_order lê INVENTORY mas nunca escreve nele nem imprime nada, então você pode chamá-la mil vezes em um teste sem resetar nada. process_order agora é o único lugar que toca INVENTORY ou print, e cabe em uma tela com espaço sobrando.
O que a função refatorada ganha sobre a original
O aninhamento passou de quatro níveis para um. Cada função faz o único trabalho que seu nome descreve. INVENTORY é escrito a partir de exatamente um lugar, e não sobra nenhum ramo copiado e colado para corrigir duas vezes. Adicionar um nível “atacado” é uma mudança de uma linha em DISCOUNT_RATES.
O histórico importa tanto quanto o código. Agora você tem cinco commits (o teste, depois quatro refactorings), cada um verde contra os mesmos testes de caracterização. Se um desconto sair errado daqui a seis meses, o git blame na linha do preço vai cair em um commit pequeno o suficiente para ler por completo. Se você esmagar os cinco em um único commit “refactor” ao fazer o merge do branch, perde isso. Git Rebase vs Merge mostra como cada estratégia de merge molda o histórico que sobra.
Quando não refatorar spaghetti code
Um teste de caracterização precisa que o comportamento atual seja observável e valha a pena preservar. Se o prazo é hoje e não há teste para se apoiar, refatorar transforma uma bagunça funcional em uma bagunça quebrada com formatação melhor. Entregue a correção, depois planeje a limpeza como uma tarefa à parte. Não a esconda dentro de um commit de correção de bug, onde ela oculta a mudança real.
Código programado para ser removido também não merece um refactoring. Se a funcionalidade será substituída no próximo sprint, as horas vão para a substituição, não para polir a versão antiga pelas duas semanas que ela ainda tem.
Reestruturar código sem nenhum teste é uma reescrita, e você não tem como saber o que quebrou. Se não consegue testar o comportamento atual, o primeiro passo é torná-lo testável.
Escolha uma função que você evita abrir, escreva um único teste de caracterização contra o que ela faz agora, e faça a primeira extração. Faça o commit antes de tocar em qualquer outra coisa.