OO, Refactoring

Refatorar é bom para aprender OO

Uncle Bob blogou no Clean Coder sobre uns code reviews que ele fez do código (e das refatorações) de um cara chamado John MacIntyre.

Uncle Bob criticou a conclusão do cara de que refatoração não vale a pena para projetos reais: refatoração é uma maneira efetiva de evitar que o código apodreça e fique difícil de manter.

O que achei de mais interessante sobre o post do Uncle Bob é que ele pegou o código inicial do cara e foi fazendo pequenas refatorações, no estilo Baby Steps. No final, acabou melhorando drasticamente a estrutura do código e tornando-o muito mais extensível. O exemplo é bem pequeno mas, mesmo assim, houve uma melhora significativa.

O código inicial:

package payroll;

public class PayCalculator {
  public static float calculate(double hours,
                                double rate,
                                boolean isHourlyWorker) {
    if (hours < 0 || hours > 80) {
      throw new RuntimeException("Hours out of range: " + hours);
    }
    float wages = 0;

    if (hours > 40) {
      float overTimeHours = hours - 40;
      if (!isHourlyWorker) {
        wages += (overTimeHours * 1.5) * rate;
      } else {
        wages += overTimeHours * rate;
      }
      hours -= overTimeHours;
    }
    wages += hours * rate;
    return wages;
  }
}

O código depois da refatoração:

package payroll;

public class ContractorCalculator extends PayCalculator {
    @Override
    protected double determinePay(double hours, double rate) {
        return hours * rate;
    }
}

public class HourlyCalculator extends PayCalculator {
    @Override
    protected double determinePay(double hours, double rate) {
        double overtime = Math.max(0, hours - 40);
        return hours * rate + overtime * rate * 0.5;
    }
}
package payroll;

public abstract class PayCalculator {
    public final double calculate(double hours, double rate) {
        validateHours(hours);
        return determinePay(hours, rate);
    }

    protected void validateHours(double hours) {
        if (hours < 0 || hours > 80) {
            throw new RuntimeException("Hours out of range: " + hours);
        }
    }

    protected abstract double determinePay(double hours, double rate);
}

Para refatorar o código original, imagino que Uncle Bob tomou os seguinte passos (de bebê):

  1. Criou uma suíte de testes para o código inicial.
  2. Simplificou um pouco o algoritmo e rodou os testes, para ver se não tinha quebrado nada.
  3. Livrou-se do boolean isHourlyWorker do método calculate, colocando-o como variável de instância da classe PayCalculator e passando o booleano no construtor. Isso fez com que os testes não compilassem.
  4. Modificou a instanciação de PayCalculator nos testes, para que compilassem. Isso ficou meio estranho, o que expôs que haviam duas classes derivadas da classe PayCalculator (uma com o parâmetro do construtor true e outra false).
  5. Criou as subclasses ContractCalculator, que passava false como parâmetro para o construtor da sua superclasse e HourlyCalculator, que passava true.
  6. Modificou a instanciação dos testes para usar as subclasses.
  7. Moveu a implementação do método calculate, ainda usando isHourlyWorker, para as subclasses.
  8. Tornou abstrato o método calculate da superclasse PayCalculator.
  9. Modificou a implementação do método calculate nas subclasses, só deixando o código correspondente as responsabilidade de cada subclasse.
  10. Removeu o booleano isHourlyWorker do construtor da classe PayCalculator e removeu a chamada das subclasses a esse construtor.

A cada passo, os testes são rodados. Parece muito complicado para um código tão pequeno, mas é muito flúido programar desse jeito. Lembra o trabalho de um sushi man que, a cada movimento, deixa sua faca e outros utensílios limpos e organizados.

O código ficou muito mais extensível: para criar um outro tipo de PayCalculator, eu não preciso modificar o código da classe PayCalculator. Através de polimorfismo e classes abstratas, criamos um ponto de extensão para o que tinha mais sentido de negócio. A esse modificar por extensão deram o nome de Open/Closed Principle. Uma métafora interessante desse princípio, que é mostrada na figura abaixo, é que para colocar um casaco você não precisa fazer uma cirurgia para abrir o peito.

Open/Closed Principle

Uma observação: métodos que recebem booleanos são mais difíceis de ler e de usar, porque em geral o programador tem que olhar o código do método para realmente saber o que o booleano faz. E usar booleanos para desviar o fluxo do código mostra que o método está fazendo duas coisas,  mais do que deveria. Além disso, se você quiser criar uma terceira coisa, você tem que trocar o booleano por um inteiro, Enum, ou coisa parecida e modificar o código do método.

Conforme foi dito em um comentário do post do Clean Coder, o código de Uncle Bob teve uma duplicação talvez desnecessária: a chamada do método validateHours()  em ambas as subclasses, tanto na ContractCalculator quanto na HourlyCalculator. Quem fez esse comentário colocou uma sugestão de código no Gist, que foi o que reproduzi acima.

No livro Refactoring, Martin Fowler faz algo parecido, pegando um código pseudo-OO, com classes anêmicas e toda a lógica em um só método,  e fez pequenas refatorações em , até tornar o código muito mais simples e modificável. Coloquei o código do exemplo no meu github.

Mas, afinal de contas, o que que tem a ver o título deste post?

É que acredito começar de um código estruturado, modificando-o em pequenos passos, até chegar a um código extensível e com as responsabilidades bem definidas, é uma boa maneira de ver a verdadeira utilidade de OO. E de aprender!

Anúncios
OO, TDD

TDD favorece a estabilidade

Mauricio Aniche escreveu um ótimo post falando sobre TDD e estabilidade.

Estabilidade é uma medida da improbabilidade de uma classe ser mudada. Se uma dada classe depende diretamente de outras classes é mais provável que ela seja mudada, do que se ela depender só de interfaces.

Mauricio conclui:

TDD faz com que o programador acople suas classes com módulos geralmente mais estáveis!

Por quê?

Porque TDD obriga você a pensar apenas no que você espera das outras classes, sem pensar ainda em uma implementação concreta.

Esses comportamentos esperados acabam geralmente se transformando em interfaces, que frequentemente se tornam estáveis.

Muito bom!

Leia mais: TDD diminui o acoplamento, mas só isso não resolve!

Build, Java

Build do Eclipse não vale!

Um anti-pattern que já vi muito por aí nos projetos Java em que participei é o Build do Eclipse.

Ao invés de usar um script de build automatizado para criar um pacote entregável (JAR, WAR, EAR), o sistema é colocado em produção simplesmente copiando os .class criados pelo Eclipse. Também são copiados o .project, .classpath e .settings e, às vezes, até as pastas de controle versão (.svn).

O Eclipse é ótimo para ajudar no desenvolvimento com seu code-completion, recompilação a cada arquivo salvo, sugestões, code snippets, atalhos de teclado e o montão de plugins. Também é excelente para refatorar de maneira eficiente e segura. E até para disparar um script de build.

Mas o Eclipse não deve ser usado como ferramenta de build.

Por que não?

  1. Porque builds tem que ser automatizados
  2. Usar builds do Eclipse, é um forte indicador de que você não automatiza seus builds.

    OK, você pode automatizar o build criando um sistema que abre o Eclipse, dispara os cliques necessários e copia os arquivos. Mas ninguém faz isso. E, convenhamos, essa ideia é estúpida. Usar uma ferramenta de build especializada como Ant ou Maven é muito, mas muito, mais simples.

    E por que automatizar seus builds é bom?

    Builds são receitas de bolo. São trabalhos repetitivos e tediosos. Não são processos criativos. Humanos são bons em criar e não em repetir.

    Com menos intervenção humana, seus builds terão menos erros. E você liberará tempo para trabalho criativo, que é o que os seus programadores deveriam estar fazendo.

  3. Porque você precisa usar sempre a mesma IDE
  4. Bons programadores Java tem opinião forte sobre qual IDE é melhor. Se você forçá-lo a usar uma IDE diferente, você fará um programador menos feliz e programadores bons precisam estar felizes para produzir código de qualidade.

    Pode parecer besteira, e talvez seja. Mas restrições de IDE são desnecessárias. Com um bom script de build, você se liberta de qualquer acoplamento com uma IDE específica e permite flexibilidade na hora de desenvolver.

  5. Porque você não está integrando continuamente
  6. Integração Contínua é uma das práticas mais interessantes (e usadas) do XP. Integrar continuamente é bom em qualquer contexto e em qualquer metodologia. E ajuda bastante na diminuição de vários riscos de projetos de software como descoberta tardia de problema de integração e defeitos, falta de código entregável e baixa qualidade interna.

    Mas para integrar continuamente você precisa ter, no mínimo, builds automatizados. Se o seu servidor de Integração Contínua precisa do Eclipse instalado, isso é um mal sinal.

    Para aproveitar todo o valor da Integração Contínua, além de builds automatizados, é importante que você tenha testes automatizados e análise estática de código. Assim, você testará seu código constantemente e evitará que código sem testes, duplicado e complexo entre em seu repositório.

Generalizando
Na verdade, um nome melhor para esse anti-pattern seria Build da IDE, porque o mesmo vale para Netbeans, IDEA ou qualquer outra IDE.

Mas, enfim, build do Eclipse não vale!

Coding Dojo

Coding Dojo @ engsoftagil

Depois de 7 horas de aula em pleno Sábado, uma galera corajosa da #engsoftagil se reuniu para um coding dojo. O problema escolhido foi o FizzBuzz. A linguagem foi Ruby. Os detalhes sobre o dojô estão no dojobh.

Tenho alguns comentários:

  1. Gostei das discussões sobre qualidade do código vs. preocupação com performance. O pessoal decidiu por deixar explícita a intenção do código, o que achei muito bom.
  2. Mesmo nesse problema muito simples (FizzBuzz), precisamos fazer muitas decisões de design ao implementar. Isso, pra mim, invalida opiniões (principalmente de alguns gerentes por aí) de que programação é algo mecânico. Espero que o pessoal tenha tido o mesmo feeling.
  3. Surgiram questões interessantes relacionadas a TDD:
    • devo escrever outro teste ou refatorar o código?
    • como sei que meus testes são suficientes?
    • para um problema com uma solução óbvia, posso escrever logo a solução que acho que é a correta e implementar os testes depois?

    Todas essas são questões pertinentes e que mostra que o pessoal estava interessado e questionando TDD, mas com a mente aberta.

Uma coisa que eu deveria ter evitado e que pretendo melhorar da próxima vez é o respeito a par que está programando. Fiquei dando pitaco e sugerindo um monte de coisa. Isso atrapalha quem está programando. Devia ter ficado mais calado.

Faltou um teste que juntasse tudo, que Rondy começou a implementar mas não teve tempo de terminar, porque o pessoal começou a Retrospectiva. Coloquei no Gist o código de um possível teste de wrap-up.

def test_fizzbuzz_de_1_a_20
      sequence = []
      1.upto(20) do |n|
           fizzbuzz = FizzBuzz.new(n)
           sequence.push(fizzbuzz.run)
       end
       assert_equal '1,2,fizz,4,buzz,fizz,7,8,fizz,buzz,11,fizz,13,14,fizzbuzz,16,17,fizz,19,buzz', sequence.join(",")
end

Houve um efeito ruim da decisão de passar o número como parâmetro do construtor, que é ter que instanciar um objeto a cada número. Acho que, como não precisamos manter estado, seria melhor passar o número em questão como parâmetro do método run, que executa a lógica do FizzBuzz.

Uma coisa interessante a se fazer, é implementar o FizzBuzz passando um Range como parâmetro. Aí, mudaria o código para retornar um Array, ao invés de um número ou String. Aí o código do teste acima iria ficar algo parecido com:

def test_fizzbuzz_de_1_a_20
       fizzbuzz = FizzBuzz.new
       sequence = fizzbuzz.run(1..20).join(",")
       assert_equal '1,2,fizz,4,buzz,fizz,7,8,fizz,buzz,11,fizz,13,14,fizzbuzz,16,17,fizz,19,buzz', sequence
end