Return Early: um opção para simplificar expressões condicionais

Há uma grande polêmica sobre ter ou não vários returns em um método. Alguns defendem que deve haver apenas um retorno. Outros defendem que deve-se retornar assim que houver um valor a devolver.

Enfim, este artigo aplica a segunda técnica, onde retornamos o mais cedo possível. Esta técnica é conhecida como:

RETURN EARLY

Dar um retorno cedo ajuda a descomplicar expressões condicionais. Bem, tudo fica melhor com um exemplo não é? Considere uma classe Seguranca, usada para computar o Nivel de Acesso de um dado Usuario. Veja a seguir:

public class Seguranca {
 
 public enum NivelAcesso {
  Nenhum, Visitante, Operador, Administrador;
 }

 public static NivelAcesso privilegio(Usuario usuario) {
  NivelAcesso nivel = null;

  if (!usuario.isBloqueado()) {
   if (usuario.getCargo() != null) {
    if (usuario.getCargo() == Usuario.Cargo.Gerente) {
     nivel = NivelAcesso.Administrador;
    } else if (usuario.getCargo() == Usuario.Cargo.Funcionario) {
     nivel = NivelAcesso.Operador;
    }
   } else {
    nivel = NivelAcesso.Visitante;
   }
  } else {
   nivel = NivelAcesso.Nenhum;
  }

  return nivel;  
 }
 // ... 

Esta implementação aplica um único ponto de saída, um único retorno no fim do método. Isto é bom, por um lado, pois facilita acompanhar o fluxo da lógica, e ruim por outro, pois começa a aninhar as expressões condicionais.

A seguir uma implementação alternativa, uma refatoração, onde é usado o return early:

public class Seguranca {
 
 public enum NivelAcesso {
  Nenhum, Visitante, Operador, Administrador;
 }

 public static NivelAcesso privilegio2(Usuario usuario) {

  if (usuario.isBloqueado()) {
   return NivelAcesso.Nenhum;
  }

  if (usuario.getCargo() == Usuario.Cargo.Gerente) {
   return NivelAcesso.Administrador;
  }

  if (usuario.getCargo() == Usuario.Cargo.Funcionario) {
   return NivelAcesso.Operador;
  }

  return NivelAcesso.Visitante;
 }

 // ... 

A mecânica do return early é simples: usar as regras mais restritivas antes, por exemplo, no caso de usuário estar bloqueado já não é necessário checar cargo e demais condições. A regra geral fica no fim, ou seja, no exemplo, se não está bloqueado e não há cargo definido então é visitante.

Para finalizar, esta técnica é independente de linguagem, mesmo que os exemplos estejam escritos em Java.

Código fonte disponível aqui: https://github.com/marciojrtorres/livro-aps/tree/master/return_early

Refatoração: substitua exceção por teste

Durante uma aula surgiu uma implementação feita por um aluno, créditos ao Matheus Cezar, onde se encaixa uma refatoração bem interessante: substituir exceção por teste.

A crônica é a seguinte: considere um método que retorne um elemento de uma coleção pelo índice ou nulo caso não exista o dado elemento. Um código cobaia pode ser visto a seguir.

public class Main {

    static String[] opcoes = {"zero", "um", "dois"};
    
    public static void main(String[] args) {
        
    }
    
    // retornar a opção ou nulo caso não seja encontrada
    static String getOpcao(int numero) {
        // implementação aqui
    }
}

Existem vários modos de cumprir a API do método, uma delas é introduzindo um mau cheiro no código, utilizando a captura de exceções como desvio condicional. É bem simples, veja a seguir:

public class Main {

    static String[] opcoes = {"zero", "um", "dois"};
        
    public static void main(String[] args) {
        System.out.println(getOpcao(9));
    }
    
    // retornar a opção ou nulo caso não seja encontrada
    static String getOpcao(int numero) {
        try {
            return opcoes[numero];
        } catch (ArrayIndexOutOfBoundsException e) {
            return null;
        }
        
    }
}

Esta é uma implementação intuitiva a primeira vista, note que ela resolve o problema, ou seja, neste exemplo é impresso null, pois como não há o índice 9 o código opcoes.get(numero) lança a exceção ArrayIndexOutOfBoundsException que é capturada e suprimida, retornando null na situações excepcionais.

Existem um detalhe importante a respeito de exceções, elas introduzem um overhead, já que cada vez que acontecem, além do bloco condicional um objeto é criado para representar a exceção.

O que eu quero dizer é que esta implementação, baseada na captura da exceção, tem baixa performance se comparada com a realização de um teste, que é a refatoração proposta e inclusive documentada no livro do Fowler.

Escrevi um pequeno benchmark para instrumentar este código, veja a seguir:

public class Main {

    static String[] opcoes = {"zero", "um", "dois"};
    
    public static void main(String[] args) {
        long inicio = System.currentTimeMillis();
        for (int i = 0; i < 10000; i++) {
            getOpcao(9);
        }
        System.out.println((System.currentTimeMillis() - inicio) + "ms");
    }
    
    // retornar a opção ou nulo caso não seja encontrada
    static String getOpcao(int numero) {
        try {
            return opcoes.get[numero];
        } catch (ArrayIndexOutOfBoundsException e) {
            return null;
        }
        
    }
}

O tempo decorrido para executar 10000 (dez mil) consultas é de 110ms no meu modesto notebook equipado com um Intel Pentium Dual Core. A refatoração a seguir torna o código mais claro e também mais rápido, é a substituição de exceção por teste, veja a seguir:

public class Main {

    static String[] opcoes = {"zero", "um", "dois"};
    
    public static void main(String[] args) {
        long inicio = System.currentTimeMillis();
        for (int i = 0; i < 10000; i++) {
            System.out.println(getOpcao(9));
        }
        System.out.println((System.currentTimeMillis() - inicio) + "ms");
    }
    
    // retornar a opção ou nulo caso não seja encontrada
    static String getOpcao(int numero) {
        if (numero >= 0 && numero < opcoes.length)        
            return opcoes[numero];        
        return null;
    }
}

Esta implementação executa em 5ms em média, bem melhor do que os 110ms anteriores, não? A mecânica é simples, em vez de confiar no catch nós fazemos um teste para verificar a validade do parâmetro.

É isso, deixe seu comentário se quiser.

Enumerados em Java

Intro

Muitas pessoas não utilizam os tipos enumerados em Java, seja por não conhecerem por estar começando a programar em Java ou, no caso dos experientes, serem relutantes a mudanças (enums foram introduzidos no Java 5).

O fato é que enums quebram um galhão e, diferente como alguns pensam, não são apenas constantes (como é no C# por exemplo), eles podem ter atributos e métodos.

Para entender a utilidade de enums vamos aplicá-los em dois estudos de caso:
1: Uma refatoração de "obsessão primitiva" (um mau cheiro)
2: Introdução do Design Pattern Strategy (Padrão de Projeto Estratégia)

Neste post vou abordar o uso de enums em refatorações e no próximo post abordo o segundo caso, ok?



Usando enums em refatorações

Imagine a seguinte implementação:

public class Usuario {

    String nome;
    String login;
    String senha;
    int    perfil = 0;

}

Não fica claro o que perfil quer dizer, fica?

Então podemos adicionar um comentário! (a.k.a. desodorante!)

public class Usuario {

    String nome;
    String login;
    String senha;
    
    // perfil do usuário pode ser:
    // 0:visitante,
    // 1:usuário,
    // 2:funcionário,
    // 3:gerente,
    // 4:administrador
    
    int    perfil = 0;

}

Há ainda os que colocam caracteres de destaque no comentário (péssimo amigo):

// --------------------------------------------------------- //
// perfil do usuário pode ser:                               //
// 0:visitante,                                              //
// 1:usuário,                                                //
// 2:funcionário,                                            //
// 3:gerente,                                                //
// 4:administrador                                           //
// --------------------------------------------------------- //
    
int    perfil = 0;

Steve McConnel, respeitado Engenheiro de Software diz:

Bom código é sua melhor documentação. Sempre que você estiver por adicionar um comentário, pergunte a si mesmo: 'Como eu posso melhorar o código para que este comentário não seja necessário?'

Antes do Java 5 a opção era usar constantes, algo como:

public class Usuario {

    String nome;
    String login;
    String senha;
    int perfil = Perfil.VISITANTE;

    public abstract class Perfil {

        public static final int VISITANTE = 0;
        public static final int USUARIO = 1;
        public static final int FUNCIONARIO = 2;
        public static final int GERENTE = 3;
        public static final int ADMINISTRADOR = 4;
    }
}

O que já é bem mais profissional mesmo que não ofereça a segurança do enum. Digo isto por causa do benefício da tipagem forte, o que faz o Java uma opção para softwares robustos. Usando a implementação com constantes não há impedimento que alguém faça:

Usuario umUsuario = new Usuario();
umUsuario.perfil = 8;

Afinal, a variável perfil é do tipo inteiro. Claro que se encapsularmos a configuração do perfil em um setter (setPerfil(int perfil)) podemos tratar o número para que seja válido. Mas vamos um passo além, usaremos enums, como abaixo:

public class Usuario {

    String nome;
    String login;
    String senha;
    Perfil perfil = Perfil.VISITANTE;

    public enum Perfil {

        VISITANTE, USUARIO,
        FUNCIONARIO, GERENTE,
        ADMINISTRADOR;
    }
}

Agora, a variável perfil é do tipo Perfil. Apenas os valores enumerados são aceitos, além de nulo é claro, pois o enumerado é um tipo (classe).

Sempre é preferível implementar utilizando a tipagem forte, que é bem viva no Java. A tipagem forte permite ver possíveis erros ainda em tempo de compilação e benefícia o código coletivo já que a intenção é declarada e a quantidade de valores aceitos é limitada. Seria diferente se usássemos Strings por exemplo, que é aberto a qualquer coisa.

Os níveis numéricos ainda podem ser obtidos, como no exemplo:

Usuario umUsuario = new Usuario();
umUsuario.perfil = Usuario.Perfil.GERENTE;

// mostra o índice
System.out.println(umUsuario.perfil.ordinal());
// mostra o nome
System.out.println(umUsuario.perfil.toString());

Simples não?

Outro caso comum é na passagem de parâmetros. Imagine um funcionário que é recuperado de um banco de dados e tem seu salário calculado assim:

Funcionario func = ds.findFuncionarioById(1288);

double salario = func.calculaSalario(3, true);

O que é 3? E o true? É para calcular de verdade? hehehe

Bem, teríamos que ver a assinatura do método para entender. Digamos que seja isto:

public double calculaSalario(int mes, boolean liquido) {
    // implementação
}

Ah! O número é o mês e o true é para calcular liquido. False calcula o quê? Deve ser o salário bruto, eu acho.

Não preciso dizer que em programação não há espaço para achismo, então vamos implementar melhor este método usando o quê? usando o quê? ENUMS!

public class Funcionario {

    public enum Mes {
        JANEIRO, FEVEREIRO, MARCO,    ABRIL,   MAIO,     JUNHO,
        JULHO,   AGOSTO,    SETEMBRO, OUTUBRO, NOVEMBRO, DEZEMBRO;
    }
    
    public enum Valor {
        LIQUIDO, BRUTO;
    }

    public double calculaSalario(Mes mes, Valor valor) {
        // implementação
        return 0.0;
    }

}

Com esta implementação agora eu posso fazer:

Funcionario func = ds.findFuncionarioById(1288);

double salario = func.calculaSalario(Mes.MARCO, Valor.LIQUIDO);

Melhor?

Bem, mas digamos que ainda queira se manter a opção de passar o mês como inteiro. Note, usar tipos primitivos permitirá que seja passado qualquer valor, ou seja, algo como calculaSalario(15), bem propenso a erros. Também é o caso de querermos o número 1 para Janeiro, 2 para Fevereiro, etc, o que não é o caso quando chamamos ordinal() que começa com zero.

Então vamos preparar nosso enum para ser instanciado a partir de um inteiro para que faça a numeração correta:

public enum Mes {

    JANEIRO(1),  FEVEREIRO(2), MARCO(3),     ABRIL(4),
    MAIO(5),     JUNHO(6),     JULHO(7),     AGOSTO(8),
    SETEMBRO(9), OUTUBRO(10),  NOVEMBRO(11), DEZEMBRO(12);
    public int Numero;

    Mes(int n) {
        this.Numero = n;
    }

    public static Mes valueOf(int index) {
        for (Mes mes : EnumSet.allOf(Mes.class)) {
            if (mes.Numero == index) {
                return mes;
            }
        }
        return null;
    }
}

O valor entre parênteses nos enumerados definem o enum passando o número no construtor. Assim sendo, se eu chamar Mes.JULHO.Numero retorna 7.

O método valueOf(int) devolve o enumerado adequado conforme o número, ou nulo se não for encontrado.

A implementação da chamada do método calculaSalario usando inteiros fica assim:

double salario = func.calculaSalario(Mes.valueOf(3), Valor.LIQUIDO);

Conclusão

Java é uma linguagem fortemente tipada e os programadores devem tirar proveito deste recurso que, naturalmente, faz do Java uma linguagem robusta, confiável e preferida por empresas de grande porte.

Primitive Obsession, Value object, Uniform Access Principle, Factory Method e Open/Close Principle

Salve,

Este tópico é para abordar um Code Smell (mau cheiro no código) chamado Primitive Obsession (obsessão primitiva). Este Code Smell é encontrado regularmente nas propriedades de classes e não é difícil identificá-lo. A refatoração comum é extrair uma classe, normalmente um Value Object (objeto de valor) imutável, criando um modelo mais rico, fazendo uso de princípios adequados de projeto orientado a objetos, como o UAP, Uniform Access Principle (Princípio do Acesso Uniforme) e OCP, Open/Close Principle (Princípio Aberto/Fechado).

Primitive Obsession

Imagine a situação onde seja necessário modelo um domínio de encomendas, tendo em mente que o valor cobrado no frente é relacionado com o peso da encomenda, pode ser? Naturalmente é comum modelarmos assim:

class Encomenda {
    int peso;
}

Obs.: deixei de lado Getter's e Setter's para favorecer a legibilidade do exemplo;

Mais tarde existirá a intenção de cobrar o frete por peso, por exemplo R$ 2,15 por Kilo. Mas o peso da encomenda usa qual unidade de medida? E se for necessário usar 0,5Kg? Pois então, o modelo deve ser refinado e talvez teremos um modelo assim:

class Encomenda {
    double pesoKilos;
}

Mais tarde é confirmado que a interface gráfica mostrará uma caixa de texto para entrar com o peso em kilos ou gramas. Se o usuário entrar com, por exemplo, 700 gramas, no evento será necessário converter estes 700 gramas para kilos, dividindo por 1000, para depois atribuir a encomenda. Então, após perceber a "gambi", talvez o modelo fique assim:

class Encomenda {
    double pesoKilos;
    int unidade; 
}

Como os outros colegas não entendem o que é essa tal de unidade em inteiro você decide então colocar um (desodorante) comentário:

class Encomenda {
    double pesoKilos;
    int unidade; // use 0 para kilo e 1 para gramas
}

Enfim, é muito comum querermos representar grupos de dados com tipos primitivos e isto é muito comum para quem vem de outras linguagens, a maioria procedural, as quais não proviam uma maneira uniforme e significativa de representar grupos ou estruturas de dados.

E como é a maneira OO de fazer? Ora, criando classes.

Aos que perguntam "bah, quantas classes vou criar?", digo: quantas forem necessárias. Programar em linguagens orientadas a objetos implica em criar classes (tipos), é fato.


Value Object (VO)

Objetos de valor são importantes para representar informações que não necessitam de uma identidade (exemplo, contrastando uma classe Compromisso com uma classe Data). Normalmente são imutáveis e fornecem métodos para tratar seu estado uniformemente. O caso do peso da encomenda é um forte candidato a ser um VO. Extraindo o peso para uma classe estaremos fazendo uma importante refatoração, para "curar" a nossa obsessão primitiva.

De início, poderíamos fazer algo mais ou menos assim:

class Encomenda {
    Peso peso;
}

class Peso {
    int valor;
}

Mas e agora, como vamos tratar esta representação de Peso. Bem, podemos fazer várias melhorias, como representar e converter facilmente unidades de medida, como Kilos, gramas e outros. O Kiko por exemplo, fez uma observação importante. Ele disse que poderíamos até mais tarde representar libras ou outra unidade estrangeira.

Primeiro temos de definir a unidade básica interna de armazenamento e depois fornecer acessores a este estado. Eu prefiro usar gramas, o que seria por exemplo, uma unidade mínima. A classe, agora completa para focar a solução, seria mais ou menos assim:

class Peso {
    private int gramas;

    public Peso(int gramas) {
        this.gramas = gramas;
    }

    public Peso(double kilos) {
        this.gramas =  (int) (1000 * kilos);
    }

    public int getGramas() {
        return gramas;
    }

    public double getKilos() {
        return gramas * 1000;
    }
    
}

Uniform Access Principle

Particularmente, ainda acho que esta classe deve ser refinada, mas ela já mostra a aplicação do Princípio do Acesso Uniforme. Este princípio baseia-se na premissa de que qualquer usuário da classe não precisa saber se a propriedade que está usando é um valor armazenado ou calculado. Se o método getKilos fosse converteEmKilos() estaríamos violando este princípio.

Do modo que está, o usuário da classe pode acessar o peso em gramas e em kilos uniformemente e transparentemente.

Eu não inclui métodos setGramas e setKilos, mas se o fizesse, o método setKilos converteria para gramas, como no construtor, e o usuário da classe ainda não teria conhecimento de como o peso internamente é armazenado (e nem é interessante). Incluir métodos set também implica em tornar o objeto mutável, a não ser que a cada set seja retornado um novo objeto Peso.


Factory Methods

Comentei que era possível fazer uma melhoria, penso que poderíamos substituir os construtores por métodos de fábrica (uma implementação simplificada do padrão de projeto Factory Method). Por quê? Imagine o caso:

Em uma instanciação da classe Peso como esta, como sabemos se é em gramas ou kilos?

Peso peso = new Peso(pesagem);

Pois é, apenas com essa linha não saberemos. Teremos que seguir a variável pesagem para saber se ela é do tipo int ou double para saber se a instância será em gramas ou kilos respectivamente.

Aplicando os métodos de fábrica, torna-se os construtores e privados e adiciona-se métodos estáticos com nomes amigáveis:

class Peso {
    private int gramas;

    private Peso(int gramas) { this.gramas = gramas; }
    
    private Peso(double kilos) { this.gramas =  (int) (1000 * kilos); }

    public static Peso comGramas(int gramas) { 
        return new Peso(gramas);
    }
    
    public static Peso comKilos(double kilos) {
        return new Peso(kilos);
    }
    
    public int getGramas() {
        return gramas;
    }

    public double getKilos() {
        return gramas / 1000;
    }
    
}

Então a chamada anterior ficaria assim:

Peso peso = Peso.comGramas(pesagem);

Desta maneira não é necessário seguir variáveis ou ler comentários para entender esta linha.



Open/Close Principle

Mas e se amanhã quisermos adicionar uma nova unidade de medida? Obviamente teremos que abrir a classe e implementar, a não ser que programemos com o princípio aberto/fechado em mente. O Princípio Aberto/Fechado diz que uma classe deve estar fechada para modificação mas deve estar aberta para extensão.

Existem vários modos de "abraçar" este princípio, um deles é usando o Design Pattern Strategy (padrão de projeto estratégia). Exemplo:

class Peso {
    private int gramas;

    private Peso(int gramas) { this.gramas = gramas; }

    private Peso(double kilos) { this.gramas =  (int) (1000 * kilos); }

    public static Peso comGramas(int gramas) {
        return new Peso(gramas);
    }

    public static Peso comKilos(double kilos) {
        return new Peso(kilos);
    }

    public static Peso comPeso(IConversorPeso conversor, double peso) {
        return new Peso(conversor.paraGramas(peso));
    }

    public int getGramas() {
        return gramas;
    }

    public double getKilos() {
        return gramas / 1000;
    }

    public double getPesoEm(IConversorPeso conversor) {
        return conversor.deGramas(gramas);
    }

    public interface IConversorPeso {

        public double deGramas(int gramas);
        public int paraGramas(double peso);

    }
    
}

Sabendo que 1 grama = 0,00220462262 libras, então poderíamos fazer isto:

class Libras implements IConversorPeso {

    public double deGramas(int gramas) { return gramas * 0.00220462262; }

    public int paraGramas(double peso) { return (int) (peso / 0.00220462262); }
}

Eu posso passar para Peso qualquer implementação de IConversorPeso e assim estender a funcionalidade da classe Peso sem alterá-la, aderindo ao Princípio Aberto/Fechado.


Caso de teste

Sabendo que 10 Kilogramas = 10000 gramas = 22,0462262 libras

Libras libras = new Libras();

System.out.println(libras.deGramas(10000));

Resultado Final

Obtendo o peso em Kilos instanciando com 500 libras:

Libras libras = new Libras();

Peso peso = Peso.comPeso(libras, 500);

System.out.println(peso.getKilos());

Conclusão

Minha implementação tem falhas de arredondamento (usa double para gramas atenuaria) mas acho que demonstra os temas. Espero que seja útil e que eu tenha me feito entender.

Abraços!

Tchê Linux: Boas práticas e Refatorações

Olá,

A apresentação no último sábado (28/ago) foi muito legal. Encontrei vários amigos, conversei com pessoas do ramo, etc. Esses encontros sempre são bons.

Na apresentação falei sobre Boas práticas e refatorações com Java, e fiz mais slides e código do que tinha de tempo disponível, para variar. Bem, o fato é que deixei disponível tanto a apresentação quanto o código que podem ser baixados nestes links abaixo:

Apresentação:
https://docs.google.com/leaf?id=0B3CElJmzM2Y6YjlmYjY2NWMtYTQ1Yi00YjIxLWExNGQtMGY1NzBkYTZiODk0&sort=name&layout=list&num=50

Código fonte:
https://docs.google.com/leaf?id=0B3CElJmzM2Y6MzlmZDNkYjQtOWQ2Mi00NDM1LWFjMjYtNTFmMTAwOGNhYWE3&sort=name&layout=list&num=50

Desculpem o atraso para disponibilizar, pois resolvi deixar uns comentários no código para facilitar o entendimento e editei o texto e Layout dos slides também.

Breve postarei mais coisas neste blog. Um abraço a todos e até a próxima, quem sabe estarei no Tchê Linux Rio Grande na FURG? Vou tentar. Até!

Refactorings: Extrair método e substituir variável temporária por consulta

Salve todos,

Como desenvolvedor, penso que uma das competências necessárias é saber refatorar o código. Isto se aprende com o tempo, e ler ajuda bastante, li o livroRefatoração: Aperfeiçoando o projeto de código existente, do Martin, e ele é muito bom, claro e coeso, demonstrando algumas refatorações consolidadas.

link livro: http://www.submarino.com.br/produto/1/242126/refatoracao:+aperfeicoando+o+projeto+de+codigo+existente

Então, como tenho um tempo livre, resolvi compartilhar algumas partes e expor para discussão aqui na lista.

Primeiro, quando refatorar?

Pode ser ...

- Quando corrige um erro;

- Quando se está adicionando uma nova funcionalidade;

- Quando está se revisando o código;

Ou, como diz Martin, na edição de seu código, na terceira vez que tiver que mexer em algo, refatore.

Segundo, é necessário refatorar?

Depende, Kent Beck diz que deve-se refatorar quando detectar mau cheiro no código. Mau cheiro, não quer dizer que algo não funcione, mas sim aquele código em que tu olhas atravessado e pensa "Dá para melhorar isso".

Abaixo, uma lista resumida de alguns maus cheiros clássicos:

- Nomes obscuros de variáveis;

- Código duplicado;

- Método muito longo;

- Classe muito longa;

- Muitos parâmetros no método;

- Construtor com nulos;

- Muitas variáveis de instância;

- Campos temporários;

- Comentários no código (a.k.a. "desodorante")

Um dos princípios da refatoração é deixar o código legível e por consequência mais manutenível. Uma refatoração que aplico com frequência, é o extrair método, que uso para substituir comentários ou reusar parte de um método para outros métodos, e alterar o nome de variáveis.

Inventei uma classe 'mock' (hehehe), por exemplo:

class PagamentoService {

   private ContabilService cs;

   public void encaminhaPagamentosCartao(Integer id) {

      //Verifica o parâmetro

      if (id == null || id <= 0) throw new IllegalArgumentException("ID invalido");

      //Localiza a venda

      Venda e = em.find(Venda.class, id);

      //Verifica se existe a venda

      if (venda == null) throw new BusinessException("Venda nao encontrada para o ID:" + id);

      //Lista de pagamentos      

      List pc = new ArrayList();

      //Verifica os que foram pagos com cartao

      for (Item i : e.itens) {

         //variável que verifica se está quitado

         boolean quitado = true;

         for (Pagamento p : i.pagamentos) {

            if (!p.faturado) quitado = false;

         }

         if (quitado) {

            for (Pagamento p : i.pagamentos) {

               if (p.isCartao) {

                  pc.add(p);

               }

            }

         }

      }

      //enviar para contabilidade

      for (Pagamento p : pc) {

         cs.send(p);

      }

   }

}

Para ficar mais legível, poderíamos deixá-la assim:

class PagamentoService {

   private ContabilService contabilService;

   public void encaminhaPagamentosCartao(Integer idVenda) {

      validaParametro(idVenda);

      

      Venda venda = localizaVenda(idVenda);

      enviaPagamentoParaContabilidade(venda);      

   }

   private void validaParametro(Integer idVenda) {

      if (idVenda == null || idVenda <= 0) throw new IllegalArgumentException("ID da Venda é invalido");

   }

   private Venda localizaVenda(Integet idVenda) {

      Venda e = em.find(Venda.class, id);

      

      if (venda == null) throw new BusinessException("Venda nao encontrada para o ID:" + id);

   }

   private void enviaPagamentoParaContabilidade(Venda venda) {

      for (Item item : venda.itens) {

         if (estaQuitado(item)) {

            for (Pagamento pagamento : item.pagamentos) {

               if (pagamento.isCartao) {

                  enviaParaContabilidade(pagamento);

               }

            }

         }

      }

   }

   private boolean estaQuitado(Item item) {      

      for (Pagamento pagamento : item.pagamentos) {

         if (!pagamento.faturado) return false;

      }

      return true;

   }

   private void enviaPagamento(Pagamento pagamento) {

      contabilService.send(pagamento);

   }

}

Geralmente a refatoração adiciona indireção, obviamente, mas vai ao encontro do paradigma orientado a objetos, que prega a utilização de pequenas partes reutilizáveis de código, sempre pensando no futuro. Neste exemplo, os comentários foram substituídos por métodos  (extrair método) que falam por si, as variáveis foram renomeadas e uma delas trocada por uma consulta (substituir variável temporária por consulta).

Como benefício, futuros métodos públicos podem utilizar os métodos privados para operações semelhantes. 

O difícil é saber até onde refatorar, dividir em pequenas peças, mas isto vem com a experiência. Após acostumar-se a refatorar, passa a não se ter mais medo de alterar o código, mas deve ser feito com cuidado para não introduzir bug's, e aí entra a importância dos testes. Geralmente a refatoração é associada com alguma prática de TDD.

Em outro tópico lanço outro estudo de caso e refatorações.

Para os analistas e DBA's de plantão, sugiro este ótimo livro: http://www.agiledata.org/essays/databaseRefactoring.html

Li alguns pequenos trechos, já que estou mais ativo no código, e acho que vale a pena.

Refactorings: Introduzir objeto parâmetro.

Salve

Direto ao ponto, imagine uma classe método que utilizem muitos parâmetros, por exemplo:

public class AluguelService {


    public List findAluguel(Date dataInicialContrato, Date dataFinalContrato,
                            Date dataInicialOcupacao, Date dataFinalOcupacao) {
        // pesquisa e regras de negócio aqui
    }

    public List findAluguelByStatus(Boolean status, 
                                    Date dataInicialContrato, Date dataFinalContrato,
                                    Date dataInicialOcupacao, Date dataFinalOcupacao) {
        // pesquisa e regras de negócio aqui
    }

    public List findAluguelByCliente(Integer idCliente, Date dataInicialContrato, 
                                     Date dataFinalContrato,
                                     Date dataInicialOcupacao, Date dataFinalOcupacao) {
        // pesquisa e regras de negócio aqui
    }

}

Esta classe é elegível a refatoração substituir parâmetros por objeto parâmetro: http://www.refactoring.com/catalog/introduceParameterObject.html

Neste caso, há sempre uma intervalo de tempo do contrato e ocupacao. Em primeiro lugar, poderia-se tratar os intervalos:

public class Intervalo {
    private Date dataInicial;
    private Date dataFinal;

    //Get's and Set's
}

E deixar a classe assim:

public class AluguelService {


    public List findAluguel(Intervalo contrato,
                            Intervalo ocupacao) {
        // pesquisa e regras de negócio aqui
    }

    public List findAluguelByStatus(Boolean status, Intervalo contrato,
                                    Intervalo ocupacao) {
        // pesquisa e regras de negócio aqui
    }

    public List findAluguelByCliente(Integer idCliente, Intervalo contrato,
                                     Intervalo ocupacao) {
        // pesquisa e regras de negócio aqui
    }

}

Acho que já deu "outra cara". Entretanto, se no domínio, o intervalo do contrato e ocupação andem sempre juntos, pode-se pensar em uni-los em um objeto parâmetro.

public class IntervaloAluguel {
    private Intervalo contrato;
    private Intervalo ocupacao;

    //Get's and Set's
}

Então, a classe ficaria assim:

public class AluguelService {


    public List findAluguel(IntervaloAluguel intervalo) {
        // faz pesquisa aqui
    }

    public List findAluguelByStatus(Boolean status, IntervaloAluguel intervalo) {
        // faz pesquisa aqui
    }

    public List findAluguelByCliente(Integer idCliente, IntervaloAluguel intervalo) {
        // faz pesquisa aqui
    }

}

Em uma prática mais Domain Driven Design poderia aplicar validações dos parâmetros nas classes parâmetro, por exemplo, tratar o IntervaloAluguel para que as datas sejam válidas, que a data de ocupacao esteja dentro do intervalo contrato, e assim por diante, deixando o a classe mais como um Value Object (não confundir com Transfer Object), se quiserem saber mais: http://www.infoq.com/presentations/Value-Objects-Dan-Bergh-Johnsson