[Refatorando Tudo!] Não use parâmetros como retorno

Sabe quando você precisa encontrar aquele bug demoníaco e não faz a menor ideia de onde ele está? Ai você começa a debugar o código e põe break point em tudo que é lugar pra tentar entender o que está acontecendo. A pior coisa que você pode encontrar são métodos com efeitos colaterais.

Dano Colateral

Imagine que vamos processar a criação de um pedido de compras. Para isso precisamos calcular o valor do frete e verificar se o usuário fazendo a compra faz parte do programa premium da empresa para marcar o produto como entrega expressa. O código atual parece algo assim:

class PedidoController
  def criar_pedido
    pedido = Pedido.criar_pedido_do_carrinho
    usuario = Usuario.buscar_por_id(params.id_usuario)
    valor_frete = calcular_frete(pedido, usuario)
  end
# ...
  def calcular_frete(pedido, usuario)
    frete = pedido.valor_total > VALOR_COM_FRETE_GRATIS ?
              0 : pedido.valor_total * 0.10
    pedido.entrega_expressa = usuario.premium? && frete == 0
    transportadoras.enviar_aviso_entrega_expressa(pedido) if pedido.entrega_expressa?
    pedido.remover_produtos_estoque
    frete
  end

Vamos entender o que está acontecendo no método calcular_frete:

Primeiro o valor do frete é definido baseado no preço do pedido.

Depois marcamos o produto como entrega expressa e enviamos essa informação para as transportadoras, dependendo do valor do frete calculado antes.

Por fim os produtos do pedido são removidos do estoque e o valor do frete é retornado.

Bom, só pela explicação já deu pra perceber que esse método faz coisa demais né? Mas o principal problema, que provavelmente vai nos pegar no futuro são as alterações no objeto produto que acontecem dentro de um método chamado calcular_frete.

Imagine que a lógica para marcar o pedido como entrega expressa está errada e você precisa corrigi-lá. Não é intuitivo achar que no método calcular_frete o pedido é marcado como entrega expressa. Esse é o principal problema de métodos com efeitos colaterais.

Identificando qual anti-padrão estamos lidando

A regra mais óbvia para se seguir é:

Não altere objetos que são passados como parâmetro

Mas, uma vez que estamos nessa situação, como sair dela? Para isso precisamos entender o motivo de o método está alterando os parâmetros. Aqui vão algumas possibilidades:

Violando o Princípio da Responsabilidade Única

Se você não conhece o Princípio da Responsabilidade Única (Single Responsibility Principle), veja esse outro post que tem mais detalhes.

Como falado anteriormente, apesar de ser um exemplo simples, só pela explicação já fica claro a quantidade de coisas que o método calcular_frete faz é grande demais para um método só.

No entanto eu acho difícil seguir esse princípio pois definir o que é uma responsabilidade é muito subjetivo. Processar pagamento é uma responsabilidade? Ou será que buscar os produtos de um pedido deveria ser uma responsabilidade separada?

Tentando dividir as responsabilidades, podemos ter o seguinte cenário:

class PedidoController
  def criar_pedido
    pedido = Pedido.criar_pedido_do_carrinho
    usuario = Usuario.buscar_por_id(params.id_usuario)
    valor_frete = calcular_frete(pedido, usuario)
    marcar_pedido_expresso(pedido, usuario, valor_frete)
    enviar_aviso_entrega_expressa(pedido)
    pedido.remover_produtos_estoque
  end

  def calcular_frete(pedido, usuario)
    pedido.valor_total > VALOR_COM_FRETE_GRATIS ?
              0 : pedido.valor_total * 0.10
  end

  def marcar_pedido_expresso(pedido, usuario, frete)
    pedido.entrega_expressa = usuario.premium? && frete == 0
  end

  def enviar_aviso_entrega_expresa(pedido)
    transportadoras.enviar_aviso_entrega_expressa(pedido) if pedido.entrega_expressa?
  end
end

As responsabilidades estão bem separadas agora, mas será que realmente melhoramos o design da aplicação? Olhe o tamanho do método criar_pedido agora, ainda continuamos alterando um parâmetro no método marcar_pedido_expresso e será que esses outros métodos realmente fazem parte do PedidoController?

Ter o princípio em mente é uma boa maneira de guiar a refatoração, mas na prática vale mais a pena combiná-lo com outros princípios para decidir como refatorar o código.

Objetos com Inveja

O code smell Feature Envy mostra uma situações onde, para implementar certa funcionalidade, um objeto precisa consulta muito mais outros objetos do que a si mesmo.

Uma maneira bem fácil de perceber essa situação é olhar o método sem focar em uma linha específica. Volte ao código anterior e tente olhar todo o corpo do método buscando padrões e repetições.

Fica bem claro ver a quantidade de chamadas ao objeto pedido, seja para ler informações dele, passá-lo como parâmetro ou até mesmo modificá-lo. Esse é um sinal de que o PedidoController está com inveja das funcionalidades de Pedido.

Podemos refatorar o código movendo essas responsabilidades de PedidoController para Pedido:

class PedidoController
  def criar_pedido
    pedido = Pedido.criar_pedido_do_carrinho
    usuario = Usuario.buscar_por_id(params.id_usuario)
    valor_frete = calcular_frete(pedido, usuario)
    pedido.marcar_como_entrega_expressa(usuario, valor_frete)
    enviar_aviso_entrega_expressa(pedido)
    pedido.remover_produtos_estoque
  end

  def enviar_aviso_entrega_expresa(pedido)
    transportadoras.enviar_aviso_entrega_expressa(pedido) if pedido.entrega_expressa?
  end
end

class Pedido
  def marcar_como_entrega_expressa(usuario, frete)
    self.entrega_expressa = usuario.premium? && frete == 0
  end
end

Agora não temos mais o efeito colateral de marcar o pedido como entrega expressa em um lugar onde não fazia sentido. Caso essa lógica precise mudar ela está concentrada em um método dentro da classe Pedido.

Violando o Diga, não Pergunte!

No post anterior vimos o princípio Diga, não Pergunte, que tenta evitar que o comportamento dos objetos fique separado das informações.

Se o código consulta as informações de um objeto para decidir o que fazer com ele, é sinal de que o princípio está sendo violado. Podemos mover então a responsabilidade de falar com as transportadoras para a classe pedido, já que ela possui a informação necessária:

class PedidoController
  def criar_pedido
    pedido = Pedido.criar_pedido_do_carrinho
    usuario = Usuario.buscar_por_id(params.id_usuario)
    valor_frete = calcular_frete(pedido, usuario)
    pedido.marcar_como_entrega_expressa(usuario, valor_frete)
    pedido.enviar_aviso_entrega_expressa(transportadoras)
    pedido.remover_produtos_estoque
  end
end

class Pedido
  def marcar_como_entrega_expressa(usuario, frete)
    self.entrega_expressa = usuario.premium? && frete == 0
  end

  def enviar_aviso_entrega_expresa(transportadoras)
    transportadoras.enviar_aviso_entrega_expressa(self) if entrega_expressa?
  end
end

Mesmo após todas essas refatorações ainda continuamos com o método criar_pedido tendo muitas responsabilidades e acionando bastante o objeto pedido, apesar de que agora ele não o modifica diretamente.

Como poderíamos melhorar o design desse código? Pense um pouco em como você melhoraria esse código e deixa as suas ideias nos comentários abaixo.

Compartilhe o texto com seus colegas para ajudá-los a melhorar o código deles também e continue acompanhando os posts da série [Refatorando tudo!] para melhorar ainda mais seu código!

Deixe seus comentário aqui no post ou fale direto comigo no twitter em @marcosbrizeno!

 

 

Anúncios

Deixe um comentário

Preencha os seus dados abaixo ou clique em um ícone para log in:

Logotipo do WordPress.com

Você está comentando utilizando sua conta WordPress.com. Sair / Alterar )

Imagem do Twitter

Você está comentando utilizando sua conta Twitter. Sair / Alterar )

Foto do Facebook

Você está comentando utilizando sua conta Facebook. Sair / Alterar )

Foto do Google+

Você está comentando utilizando sua conta Google+. Sair / Alterar )

Conectando a %s