BT

Disseminando conhecimento e inovação em desenvolvimento de software corporativo.

Contribuir

Tópicos

Escolha a região

Início Artigos Refatoração de sistemas legados: um estudo de caso

Refatoração de sistemas legados: um estudo de caso

Favoritos

O código legado sempre cheira mal. E todo desenvolvedor que se preza, ao se deparar com um código assim, tem o desejo de refatorá-lo. O ideal é refatorar o código legado com apoio de uma ferramenta de testes unitários para evitar regressão do código. Contudo, escrever testes unitários para código legado não é nada fácil; o código legado geralmente é muito bagunçado.

Para escrever testes unitários efetivos para o código legado, provavelmente será necessário refatorá-lo antes; para tal são necessários testes unitários para se certificar que nada seja quebrado. Ou seja, é a situação do ovo e da galinha - o que vem antes? Este artigo descreve uma metodologia para refatorar o código legado de maneira segura, através da apresentação da experiência do autor em um caso real.

Definindo o problema

Neste artigo, faço uso de um caso real para demonstrar as melhores práticas de teste e refatoração de sistemas legados. O sistema era em Java, mas as práticas podem ser aplicáveis em qualquer outra linguagem. Modifiquei o cenário para não expor ninguém, e simplifiquei os casos para facilitar o entendimento.

Este artigo não tem a pretensão de abordar os fundamentos de testes unitários e refatoração. Para tal, recomendo a leitura de livros como "Refactoring: Improving the Design of Existing Code" de Martin Fowler e "Refactoring to Patterns" de Joshua Kerievsky. Em vez disso, este artigo mostra um pouco das dificuldades frequentemente encontradas na vida real e fornece, assim espero, práticas úteis para superar tais dificuldades.

O exemplo usado aqui se baseia num sistema fictício de gerenciamento de recursos em que um recurso se refere a uma pessoa que pode ser designada para uma ou mais tarefas. Digamos que um recurso possa ser designado para um tíquete - um tíquete de RH ou um tíquete de TI. Um recurso também pode ser designado para uma requisição - de RH ou de TI. O gerente do recurso também pode registrar uma estimativa de tempo que o recurso deve gastar na tarefa. Por fim, o recurso registra quanto tempo foi gasto de fato num tíquete ou numa requisição. Veja a figura abaixo:

A utilização dos recursos pode ser demonstrada num gráfico de barras, que apresenta tanto o tempo estimado como o tempo real gasto:

Nada complicado, certo? Bem, no sistema real, os recursos podiam ser designados para vários tipos de tarefas, o que ainda não faria disso um modelo complexo. Entretanto, quando li o código pela primeira vez, parecia que tinha descoberto um fóssil, pois eu conseguia enxergar como o código tinha evoluído (ou melhor, involuído). Inicialmente, o sistema era capaz de lidar somente com as requisições. Posteriormente, foi adicionada a funcionalidade de lidar com tíquetes e outros tipos de tarefas.

Começou assim. Um engenheiro veio e escreveu o código para lidar com as requisições extraindo os dados do repositório e fazendo a montagem para apresentá-los no gráfico de barras, sem ao menos se incomodar em organizar a informações em objetos apropriados.

class ResourceBreakdownService {

public Map search (Session context) throws SearchException {

// ... omitidas por volta de vinte linhas de código para tirar o critério de pesquisa do contexto e verificá-los, tal como abaixo:

if(resourceIds==null || resourceIds.size ()==0) {

throw new SearchException(“Lista de Recursos não foi fornecida”);

}

if(resourceId!=null || resourceIds.size()>0){

resourceObjs=resourceDAO.getResourceByIds(resourceIds);

}

// realiza a carga de trabalho para todas as requisições

Map requestBreakDown = getResourceRequestsLoadBreakdown(resourceObjs,startDate,finishDate);

return requestBreakDown;

}

 

}

Sem dúvida esse é um código que cheira mal. Definitivamente search não é um nome representativo para o método; para testar uma coleção era melhor ter usado o método CollectionUtil.isEmpty() da biblioteca Apache Commons. E por que retornar um Map?

O mal vai além. Veio um segundo engenheiro, que seguiu o exemplo do anterior e lidou com os tíquetes do mesmo jeito. E o código ficou assim:

// realiza a carga de trabalho para todos os tíquetes
Map ticketBreakdown = getResourceRequestsLoadBreakdown(resourceObjs, startDate, finishDate, ticketSeverity);


Map result = new HashMap();

for (Iterator i = resourceObjs.iterator(); i.hasNext();) {
   Resource resource = (Resource) i.next();      

   Map requestBreakdown2 = (Map) requestBreakdown.get(resource);

   List ticketBreakdown2 = (List) ticketBreakdown.get(resource);

   Map resourceWorkloadBreakdown = combineRequestAndTicket(requestBreakdown2, ticketBreakdown2);

   result.put(resource, resourceWorkloadBreakdown);

}

 

return result;


Deixando de lado questões sobre a nomenclatura, o equilíbrio da estrutura e demais aspectos estéticos, o que mais cheira mal nesse código é o retorno do método ser um objeto Map. Um Map é um buraco negro que aceita de tudo e não fornece muitas pistas sobre o que tem dentro de si. Somente escrevendo um código de depuração para imprimir recursivamente o conteúdo do Map pude descobrir como era sua estrutura de dados.

No exemplo a seguir, {} representa um Map; => representa o valor de uma chave de mapeamento; e [] representa uma coleção:

{resource with id 30000 => [
SummaryOfActualWorkloadForRequestType,
SummaryOfEstimatedWorkloadForRequestType,
{30240 => [
        ActualWorkloadForReqeustWithId_30240,
        EstimatedWorkloadForRequestWithId_30240 ],

        30241 => [

            ActualWorkloadForReqeustWithId_30241,

            EstimatedWorkloadForRequestWithId_30241 ]

}

SummaryOfActualWorkloadForTicketType,

SummaryOfEstimatedWorkloadForTicketType,

{20000 => [

        ActualWorkloadForTicketWithId_2000,

        EstimatedWorkloadForTicketWithId_2000 ],

}     

]

 

}

Com uma estrutura de dados péssima como essa, a lógica de empacotar e desempacotar os dados estava quase ilegível, além de ser extremamente longa e tediosa.

Teste de integração

Até aqui espero ter convencido a todos que esse código era realmente complexo. Se eu tivesse começado tentando desemaranhar essa bagunça e entender cada pequeno pedaço do código antes de refatorá-lo... poderia ter ficado louca. Para preservar minha sanidade, decidi que para entender a lógica do código seria melhor usar uma abordagem de cima para baixo. Ou seja, ao invés de ler o código e deduzir a lógica, decidi que era melhor rodar o sistema e depurá-lo, e assim entender o quadro geral.

Uso essa mesma abordagem ao escrever testes. O senso comum manda escrever pequenos testes para verificar cada parte do código. Desse modo, se tudo estiver bem testado, quando todas as partes forem colocadas juntas, haverá grandes chances de que o código inteiro funcione como esperado.

Mas essa abordagem não se aplica aqui. A classe ResourceBreakdownService era uma Classe Deus (God Class). Se eu começasse quebrando a classe em pedaços, com somente o entendimento do quadro geral, teria cometido muitos erros. Isso porque poderia haver diversos segredos escondidos em cada canto do sistema legado.

Então escrevi este teste simples, que refletia o meu entendimento geral:

 

public void testResourceBreakdown() {
   Resource resource = createResource();
   List requests = createRequests();
   assignRequestToResource(resource, requests);
   List tickets = createTickets();
   assignTicketToResource(resource, tickets);
   Map result = new ResourceBreakdownService().search(resource);
   verifyResult(result,resource,requests,tickets);
}

Considere o método verifyResult(); primeiramente descobri a estrutura de result (o Map retornado pela classe ResourceBreakdownService), imprimindo seu conteúdo recursivamente. Assim verifyResult() pôde usar esse conhecimento para verificar se o objeto continha os dados corretos:

private void verifyResult(Map result, Resource rsc, List<Request> requests, List<Ticket> tickets) {

       assertTrue(result.containsKey(rsc.getId()));

       // neste caso de teste simples, a carga de trabalho real está vazia

       UtilizationBean emptyActualLoad=createDummyWorkload();        

       List resourceWorkLoad=result.get(rsc.getId());   


       UtilizationBean scheduleWorkload=calculateWorkload(rsc,requests);

       assertEquals(emptyActualLoad,resourceWorkLoad.get(0));       

       assertEquals(scheduleWorkload,resourceWorkLoad.get(1));                       


       Map requestDetailWorkload = (Map)resourceWorkLoad.get(3);

       for (Request request : requests) {

           assertTrue(requestDetailWorkload.containsKey(request.getId());

           UtilizationBean scheduleWorkload0=calculateWorkload(rsc, request);

           assertEquals(emptyActualLoad, requestDetailWorkload.get(request.getId()).get(0));

           assertEquals(scheduleWorkload0, requestDetailWorkload.get(request.getId()).get(1));               

}

// ... código para checar tíquetes omitido

 

}


Contornado os obstáculos

O caso de teste acima parece simples, porém encobre diversas complexidades. Para começar, o método ResourceBreakdownService.search() é muito acoplado ao ambiente de execução, pois tem de acessar o banco de dados e provavelmente outros serviços desconhecidos. Como acontece com vários legados, esse sistema não tinha nenhum teste unitário de infraestrutura construído. Para acessar os serviços de tempo de execução, a única opção era iniciar o sistema inteiro, o que era muito custoso e inconveniente.

A classe que inicializava a parte servidor do sistema era ServerMain. Essa classe era um fóssil também; olhando para ela era possível dizer como evoluiu. O sistema foi escrito há mais de dez anos. Naquela época não havia Spring, nem Hibernate, e JBoss e Tomcat ainda estavam engatinhando. Os bravos pioneiros daquela época tiveram que fazer muito serviço de infraestrutura "na mão", tanto que criaram um cluster "caseiro", e um serviço de cache e um pool de conexões do zero, além de diversas outras coisas.

Posteriormente, plugaram de alguma forma o sistema no JBoss e no Tomcat. Porém, infelizmente deixaram para trás tudo aquilo que haviam criado manualmente - tanto que o código foi deixado com dois conjuntos de mecanismos de gerenciamento de transações e três tipos de pools de conexões.

Decidi, então, fazer uma cópia da classe ServerMain para TestServerMain e fazer a chamada do método TestServerMain.main(). Surgiu o seguinte erro:

org.springframework.beans.factory.BeanInitializationException: Could not load properties; nested exception

is

java.io.FileNotFoundException: class path resource [database.properties] cannot be opened because it does not exist

at

org.springframework.beans.factory.config.PropertyResourceConfigurer.postProcessBeanFactory (PropertyResourceConfigurer.java:78)

É um erro fácil de corrigir! Era só pegar um arquivo database.properties, jogá-lo no classpath de teste e rodar o teste novamente. Desta vez a exceção lançada foi a seguinte:

 

java.io.FileNotFoundException: .\server.conf (The system cannot find the file specified)
   at java.io.FileInputStream.open(Native Method)
   at java.io.FileInputStream.<init>(FileInputStream.java:106)
   at java.io.FileInputStream.<init>(FileInputStream.java:66)
   at java.io.FileReader.<init>(FileReader.java:41)
   at com.foo.bar.config.ServerConfigAgent.parseFile(ServerConfigAgent.java:1593)
   at com.foo.bar.config.ServerConfigAgent.parseConfigFile(ServerConfigAgent.java:1720)
   at com.foo.bar.config.ServerConfigAgent.parseConfigFile(ServerConfigAgent.java:1712)
   at com.foo.bar.config.ServerConfigAgent.readServerConf(ServerConfigAgent.java:1581)
   at com.foo.bar.ServerConfigFactory.initServerConfig(ServerConfigFactory.java:38)
   at com.foo.bar.util.HibernateUtil.setupDatabaseProperties(HibernateUtil.java:207)
   at com.foo.bar.util.HibernateUtil.doStart(HibernateUtil.java:135)
   at com.foo.bar.util.HibernateUtil.<clinit>(HibernateUtil.java:125)

Agora era necessário colocar o arquivo serv.conf em algum lugar. Mas essa abordagem era desconfortável. Só de escrever esse único caso de teste, de imediato um problema no código ficou evidente. HibernateUtil, conforme indica o nome, deveria apenas tratar a informação de banco de dados a ser fornecida por database.properties. Por que essa classe deveria acessar server.conf, que é responsável por configurar os servidores?

Havia portanto um sinal de códigos mal-cheirosos: se ao ler um código a pergunta "por quê" está sempre na cabeça, como se estivesse lendo um romance policial, então provavelmente o código está ruim. Eu poderia ter investido mais tempo estudando os códigos de ServerConfigFactory, HibernateUtil e ServerConfigAgent, e tentar descobrir como apontar HibernateUtil para o database.properties, mas a essa altura o que queria mesmo era ver o servidor rodando. Além disso, havia uma forma de contornar a situação: usando AspectJ.

void around():
   call(public static void com.foo.bar.ServerConfigFactory.initServerConfig()) {

   System.out.println("bypassing com.foo.bar.ServerConfigFactory.initServerConfig");
}

Para quem não conhece o AspectJ, o código acima realiza o seguinte: quando a execução estiver no ponto da chamada de ServerConfigFactory.initServerConfig(), uma mensagem será impressa no console e a execução será retornada, sem entrar no método mencionado.

Isso pode parecer magia negra, mas é muito eficiente. Sistemas legados sempre são cheios de enigmas e problemas, e existem ocasiões em que é preciso partir para um combate estratégico. Naquele momento, para conseguir deixar o cliente satisfeito, o mais importante era corrigir os defeitos no sistema e melhorar a performance. Consertar coisas em outras áreas não era prioridade. Mas deixei anotado na minha lista de coisas a fazer para voltar depois e arrumar a bagunça em ServerMain.

Em seguida, no ponto em que HibernateUtil realiza a leitura das informações requisitadas de server.conf, fiz uma alteração para que as buscasse em database.properties:

String around(): call(public String com.foo.bar.config.ServerConfig.getJDBCUrl()) {
   // código omitido, lendo de database.properties

}

String around():call(public String com.foo.bar.config.ServerConfig.getDBUser()) {

   // código omitido, lendo de database.properties

 

}

Já se pode adivinhar o resto agora: contornar os obstáculos quando isso for mais conveniente ou natural a se fazer. Porém, se existem mockups já prontos, por que não reutilizá-los? Por exemplo, num dado ponto, TestServerMain.main() gerou este erro:

- Factory name: java:comp/env/hibernate/SessionFactory

- JNDI InitialContext properties:{}

- Could not bind factory to JNDI
javax.naming.NoInitialContextException: Need to specify class name in environment or system property, or as an applet parameter, or in an application resource file: java.naming.factory.initial

   at javax.naming.spi.NamingManager.getInitialContext (NamingManager.java:645)

   at javax.naming.InitialContext.getDefaultInitCtx (InitialContext.java:288)

Isso aconteceu porque o serviço de nomes do JBoss não havia sido iniciado. Eu poderia ter usado a mesma técnica de "magia negra" para contornar o problema, mas InitialContext é uma interface do Javax com muitos métodos, e fazer o mesmo para cada método seria muito trabalhoso. Pesquisando rapidamente, descobri que o Spring já tem uma classe mock SimpleNamingContext, que usei dentro do teste:

 

SimpleNamingContextBuilder builder = new SimpleNamingContextBuilder();
builder.bind(“java:comp/env/hibernate/SessionFactory”,sessionFactory);
builder.activate();

Após algumas rodadas, eu já era capaz de fazer TestServerMain.main() executar com sucesso. Comparando com ServerMain, isso foi muito mais simples, pois simulou diversos serviços do JBoss sem dificultar o gerenciamento de clusters.

Criando os blocos de construção

A classe TestServerMain está conectada a um banco de dados real, e sistemas legados podem esconder lógicas supreendentes em stored procedures - ou pior ainda, em triggers. Seguindo a mesma ideia de considerar o quadro geral, percebi que não seria boa ideia, naquele momento, tentar entender os mistérios de dentro do banco de dados e criar um mock desse banco. Então decidi fazer meus casos de teste acessarem o banco de dados real.

O caso de teste precisaria rodar repetidas vezes para garantir que cada pequena mudança feita no código do produto passasse no teste. Em cada execução, os testes criariam recursos e requisições no banco de dados. Diferentemente da prática comum em testes unitários, algumas vezes não é desejável limpar o "campo de batalha" a cada execução, eliminando dados criados pelos casos de teste.

Até aqui, o exercício de refatoração e testes tinha sido uma missão de investigação - realizar os testes para entender o sistema legado. Era possível verificar a geração dos dados através de casos de testes no banco de dados, ou usar os dados no sistema em execução para confirmar se tudo está funcionando como esperado. Ou seja, os casos de testes precisavam criar entidades únicas no banco de dados para evitar interferências com outros casos de testes. Além disso, era desejável que houvesse classes utilitárias para criar essas entidades facilmente.

Segue um exemplo de um bloco simples para criar resource:

public static ResourceBuilder newResource (String userName) {

   ResourceBuilder rb = new ResourceBuilder();

   rb.userName = userName + UnitTestThreadContext.getUniqueSuffix();

   return rb;

}

public ResourceBuilder assignRole(String roleName) {

   this.roleName = roleName + UnitTestThreadContext.getUniqueSuffix();

   return this;

}

public Resource create() {

   ResourceDAO resourceDAO = new ResourceDAO(UnitTestThreadContext.getSession());

   Resource rs;

   if (StringUtils.isNotBlank(userName)) {

       rs = resourceDAO.createResource(this.userName);

   } else {

       throw new RuntimeException("must have a user name to create a resource");

   }

   if (StringUtils.isNotBlank(roleName)) {

       Role role = RoleBuilder.newRole(roleName).create();

       rs.addRole(role);

   }

   return rs;

}

public static void delete(Resource rs, boolean cascadeToRole) {

   Session session = UnitTestThreadContext.getSession();

   ResourceDAO resourceDAO = new ResourceDAO(session);

   resourceDAO.delete(rs);


   if (cascadeToRole) {

       RoleDAO roleDAO = new RoleDAO(session);

       List roles = rs.getRoles();

       for (Object role : roles) {

           roleDAO.delete((Role)role);

       }

   }

 

}


ResourceBuilder é uma implementação combinada dos design patterns Builder e Factory:

 

ResourceBuilder.newResource(“Tom”).assignRole(“Developer”).create();

Essa classe também contém um método de limpeza: delete(). Na fase anterior desse exercício de refatoração, não invoquei delete() com muita frequência, porque eu iniciava o sistema várias vezes e obtinha os dados de teste para checar se o gráfico de barras estava correto.

Uma classe muito útil aqui foi UnitTestThreadContext, que armazena um objeto específico para threads do objeto Session do Hibernate. Ela fornece strings únicas para anexar ao nomes das entidades a serem criadas, de forma a garantir a exclusividade das entidades.

public class UnitTestThreadContext {

   private static ThreadLocal<Session> threadSession=new ThreadLocal<Session>();

   private static ThreadLocal<String> threadUniqueId=new ThreadLocal<String>();

   private final static SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy/MM/dd HH_mm_ss_S");

   public static Session getSession(){>

   Session session = threadSession.get();

   if (session==null) {

       throw new RuntimeException("Hibernate Session not set!");

   }

   return session;

}


public static void setSession(Session session) {

   threadSession.set(session);

}

public static String getUniqueSuffix() {

   String uniqueId = threadUniqueId.get();

   if (uniqueId==null){

       uniqueId = "-"+dateFormat.format(new Date());

       threadUniqueId.set(uniqueId);

   }

   return uniqueId;

}

...

 

}


Concluindo

Nesse ponto eu já conseguia iniciar uma infraestrutura de execução mínima e rodar meu caso de teste simples:

protected void setUp() throws Exception {

   TestServerMain.run(); //configura uma infraestrutura de execução mínima

}

public void testResourceBreakdown(){

   Resource resource=createResource(); //usar ResourceBuilder para construir recursos únicos

   List requests=createRequests(); //usar RequestBuilder para construir requisições únicas

   assignRequestToResource(resource, requests);

   List tickets=createTickets(); //usar TicketBuilder para construir tíquetes únicos

   assignTicketToResource(resource, tickets);

   Map result=new ResourceBreakdownService().search(resource);

   verifyResult(result);   

}

protected void tearDown() throws Exception {

   // usar TicketBuilder.delete() para deletar os tíquetes

   // usar RequestBuilder.delete() para deletar as requisições

   // usar ResourceBuilder.delete() para deletar os recursos

 

}


Continuei escrevendo mais casos de testes complexos e refatorei o código do produto e o código de teste ao longo do caminho.

De posse desses casos de testes, continuei dividindo a Classe Deus ResourceBreakdownService pouco a pouco. Não seria conveniente abordar aqui os detalhes; existem diversos livros que podem ensinar como fazer refatoração com segurança. Aqui está a estrutura resultante após a refatoração:

Agora aquela terrível estrutura de dados Map_Of_Array_Of_Map_Of... está organizada dentro de ResourceLoadBucket, que faz uso do design pattern Composite. Essa classe contém o esforço estimado e o esforço atual em um determinado nível; os esforços do próximo nível podem ser agregados pelo método aggregate(). O código resultante é muito mais limpo e tem desempenho melhor - mesmo com a exposição de alguns defeitos escondidos na complexidade do código original. E, claro, fui evoluindo os casos de testes ao longo do processo.

Através de todo esse exercício, o princípio fundamental respeitado foi o de visualizar o quadro geral. Encarei o desafio e procurei me manter focada nisso, contornando os obstáculos que não eram importantes para a atividade a ser realizada. Também construí uma infraestrutura de testes minimamente viável, que minha equipe continuou usando para refatorar outras áreas. Algumas partes de "magia negra", porém, ainda permanecem na infraestrutura de testes, porque não há muito sentido para o negócio arrumá-las.

Com tudo isso, consegui refatorar uma área funcional muito complexa e também ganhei conhecimento mais profundo sobre o sistema legado. Tratar uma aplicação legada como se fosse frágil como uma porcelana chinesa não torna o trabalho mais seguro. Ao invés disso, avançar sobre o sistema de maneira mais firme e refatorá-lo é o que deve ser feito para prolongar a vida das aplicações legadas.


Sobre a autora

Chen Ping mora em Shanghai na China e formou-se Mestre em Ciência da Computação em 2005. Desde então, trabalhou na Lucent and Morgan Stanley, e hoje atua na HP como gerente de Desenvolvimento. No tempo livre, gosta de estudar medicina chinesa.

Avalie esse artigo

Relevância
Estilo/Redação

Conteúdo educacional

  • Teoria e Pratica deveriam estar lado a lado

    by Edson Alves,

    Seu comentário está aguardando aprovação dos moderadores. Obrigado por participar da discussão!

    Gostei bastante da abordagem, em projetos que trabalhei a refatoração normalmente se resume a escrita de um novo projeto e migração da base de dados legada, vou aplicar essas boas praticas em uma oportunidade futura.

HTML é permitido: a,b,br,blockquote,i,li,pre,u,ul,p

HTML é permitido: a,b,br,blockquote,i,li,pre,u,ul,p

BT