Quanta lógica nos Getters

46

Meus colegas de trabalho me dizem que deve haver o mínimo de lógica possível em getters e setters.

No entanto, estou convencido de que muitas coisas podem ser ocultadas nos getters e setters para proteger os usuários / programadores dos detalhes da implementação.

Um exemplo do que faço:

public List<Stuff> getStuff()
{
   if (stuff == null || cacheInvalid())
   {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

Um exemplo de como o trabalho me diz para fazer as coisas (elas citam 'Código Limpo' do tio Bob):

public List<Stuff> getStuff()
{
    return stuff;
}

public void loadStuff()
{
    stuff = getStuffFromDatabase();
}

Quanta lógica é apropriada em um setter / getter? Qual é a utilidade de getters e setters vazios, exceto uma violação da ocultação de dados?

Maarten van Leunen
fonte
6
Isso parece mais com tryGetStuff () para mim ...
Bill Michell
16
Este não é um 'getter'. Esse termo é usado para o acessador de leitura de uma propriedade, não para um método que você acidentalmente coloca um 'get' no nome.
Boris Yankov
6
Não sei se esse segundo exemplo é um exemplo justo desse livro de códigos limpo que você mencionou ou se alguém está entendendo errado, mas uma coisa que não é uma bagunça quebradiça é o código limpo.
Jon Hanna
@BorisYankov Bem ... o segundo método é. public List<Stuff> getStuff() { return stuff; }
R. Schmitz
Dependendo do caso de uso exato, eu gosto de separar meu cache em uma classe separada. Faça uma StuffGetterinterface, implemente uma StuffComputerque faça os cálculos e envolva-a em um objeto de StuffCacher, responsável por acessar o cache ou encaminhar chamadas para o StuffComputerque ele envolve.
Alexander

Respostas:

71

A maneira como o trabalho diz para você fazer as coisas é esfarrapada.

Como regra geral, a maneira como eu faço as coisas é a seguinte: se pegar as coisas é computacionalmente barato ((ou se é mais provável que seja encontrado no cache)), então seu estilo de getStuff () é bom. Se conseguir que as coisas sejam conhecidas por serem computacionalmente caras, tão caras que a publicidade de seu custo é necessária na interface, então eu não chamaria de getStuff (), chamaria de calculStuff () ou algo assim, para indicar que haverá algum trabalho a fazer.

Em qualquer um dos casos, a maneira como o trabalho diz para você fazer as coisas é esfarrapada, porque getStuff () explodirá se loadStuff () não tiver sido chamado com antecedência, então eles essencialmente querem que você complique sua interface introduzindo a complexidade da ordem de operações para isso. A ordem das operações é praticamente o pior tipo de complexidade em que consigo pensar.

Mike Nakis
fonte
23
+1 por mencionar a complexidade da ordem das operações. Como solução alternativa, talvez o trabalho me peça para sempre chamar loadStuff () no construtor, mas isso também seria ruim porque significa que sempre precisará ser carregado. No primeiro exemplo, os dados são carregados lentamente apenas quando necessário, o que é o melhor possível.
laurent
6
Eu costumo seguir a regra de "se for realmente barato, use um getter de propriedade. Se for caro, use uma função". Isso geralmente me serve bem, e nomear adequadamente, como você indicou para enfatizar, também parece bom para mim.
precisa
3
se pode falhar - não é um getter. Nesse caso, e se o link do banco de dados estiver inativo?
Martin Beckett
6
+1, estou um pouco chocado com quantas respostas erradas foram postadas. Existem getters / setters para ocultar os detalhes da implementação; caso contrário, a variável deve ser tornada pública.
Izkata
2
Não esqueça que exigir que a loadStuff()função seja chamada antes da getStuff()função também significa que a classe não está abstraindo adequadamente o que está acontecendo sob o capô.
rjzii
23

A lógica nos getters está perfeitamente bem.

Mas obter dados de um banco de dados é muito mais que "lógica". Envolve uma série de operações muito caras, nas quais muitas coisas podem dar errado e de maneira não determinística. Eu hesitaria em fazê-lo implicitamente em um caso.

Por outro lado, a maioria dos ORMs suporta o carregamento lento de coleções, que é basicamente exatamente o que você está fazendo.

Michael Borgwardt
fonte
18

Eu acho que, de acordo com o 'Código Limpo', ele deve ser dividido o máximo possível, em algo como:

public List<Stuff> getStuff() {
   if (hasStuff()) {
       return stuff;
   }
   loadStuff();
   return stuff;
}

private boolean hasStuff() {
    if (stuff == null) {
       return false;
    }
    if (cacheInvalid()) {
       return false;        
    }
    return true;
} 

private void loadStuff() {
    stuff = getStuffFromDatabase();
}

Obviamente, isso é um absurdo completo, já que a bela forma que você escreveu faz a coisa certa com uma fração de código que qualquer um entende de relance:

public List<Stuff> getStuff() {
   if (stuff == null || cacheInvalid()) {
       stuff = getStuffFromDatabase();
   }
   return stuff;
}

Não deve ser a dor de cabeça do chamador de como as coisas ficam escondidas, e particularmente não deve ser a dor de cabeça do chamador lembrar-se de chamar as coisas em alguma "ordem correta" arbitrária.

Joonas Pulakka
fonte
8
-1. A verdadeira dor de cabeça ocorrerá quando o chamador estiver parado, descobrindo por que uma simples chamada getter resultou em um acesso lento ao banco de dados.
Domenic
14
@ Domínio: O acesso ao banco de dados tem que ser feito de qualquer maneira, você não está salvando o desempenho de ninguém ao não fazê-lo. Se você precisar disso List<Stuff>, só há uma maneira de obtê-lo.
DeadMG
4
@ukas: Obrigado, não me lembro de todos os truques usados ​​no código 'Clean' para tornar os bits triviais do código ainda mais uma linha ;-) Corrigido agora.
Joonas Pulakka
2
Você está difamando Robert Martin. Ele nunca expandiria uma simples disjunção booleana para uma função de nove linhas. Sua função hasStuffé o oposto do código limpo.
Kevin cline
2
Eu li o começo desta resposta e ia ignorá-la, pensando "lá vai outro adorador de livros" e, em seguida, a parte "é claro, isso é um absurdo completo" chamou minha atenção. Bem dito! C:: =
Mike Nakis
8

Eles me dizem que deve haver o mínimo de lógica possível em getters e setters.

É preciso haver toda a lógica necessária para atender às necessidades da classe. Minha preferência pessoal é o mínimo possível, mas ao manter o código, você geralmente precisa deixar a interface original com os getters / setters existentes, mas colocar muita lógica neles para corrigir a lógica de negócios mais recente (por exemplo, "clientes "o getter em um ambiente pós-911 precisa atender ao" conheça seu cliente "e aos regulamentos da OFAC , combinados com uma política da empresa que proíbe a aparência de clientes de certos países (como Cuba ou Irã).

No seu exemplo, eu prefiro o seu e não gosto da amostra "tio bob", pois a versão "tio bob" exige que os usuários / mantenedores lembrem-se de ligar loadStuff()antes de ligar getStuff()- esta é uma receita para o desastre se qualquer um de seus mantenedores esquecer (ou pior, nunca soube). A maioria dos lugares em que trabalhei na última década ainda está usando códigos com mais de uma década; portanto, a facilidade de manutenção é um fator crítico a ser considerado.

Tangurena
fonte
6

Você está certo, seus colegas estão errados.

Esqueça as regras práticas de todos sobre o que um método get deve ou não fazer. Uma classe deve apresentar uma abstração de algo. Sua classe tem legibilidade stuff. Em Java, é convencional usar métodos 'get' para ler propriedades. Bilhões de linhas de estruturas foram escritas esperando ler stuffpor telefone getStuff. Se você nomear sua função fetchStuffou algo diferente getStuff, sua classe será incompatível com todas essas estruturas.

Você pode apontá-los para o Hibernate, onde 'getStuff ()' pode fazer algumas coisas muito complicadas e lança uma RuntimeException em falha.

Kevin Cline
fonte
O Hibernate é um ORM, portanto, o próprio pacote expressa a intenção. Essa intenção não é tão fácil de entender se o pacote em si não for um ORM.
FMJaguar
@FMJaguar: é perfeitamente fácil de entender. O Hibernate abstrai as operações do banco de dados para apresentar uma rede de objetos. O OP está abstraindo uma operação de banco de dados para apresentar um objeto que possui uma propriedade nomeada stuff. Ambos ocultam detalhes para facilitar a gravação do código de chamada.
kevin cline
Se essa classe é uma classe ORM, a intenção já está expressa em outros contextos: a pergunta permanece: "Como outro programador conhece os efeitos colaterais de chamar o getter?". Se o programa contém 1k classes e 10k getters, uma política que permite chamadas de banco de dados em qualquer um deles poderia ser um problema
FMJaguar
4

Parece que isso pode ser um debate purista em relação a aplicativos que pode ser afetado pela maneira como você prefere controlar os nomes das funções. Do ponto de vista aplicado, prefiro ver:

List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

Ao contrário de:

myObject.load();
List<String> names = clientRoster.getNames();
List<String> emails = clientRoster.getEmails();

Ou ainda pior:

myObject.loadNames();
List<String> names = clientRoster.getNames();
myOjbect.loadEmails();
List<String> emails = clientRoster.getEmails();

O que tende a tornar outro código muito mais redundante e mais difícil de ler, porque você precisa começar a percorrer todas as chamadas semelhantes. Além disso, chamar funções do carregador ou similares interrompe todo o objetivo de usar o OOP, pois você não está mais sendo abstraído dos detalhes de implementação do objeto com o qual está trabalhando. Se você tem um clientRosterobjeto, não precisa se preocupar com o modo como getNamesfunciona, como faria se tivesse que chamar um loadNames, você deve saber apenas o que getNameslhe dá um List<String>com os nomes dos clientes.

Assim, parece que a questão é mais sobre semântica e o melhor nome para a função obter os dados. Se a empresa (e outras pessoas) tem um problema com o prefixo gete set, que tal chamar a função de algo parecido retrieveNames? Ele diz o que está acontecendo, mas não implica que a operação seria instantânea, como seria de esperar de um getmétodo.

Em termos de lógica em um método acessador, mantenha-o no mínimo, pois geralmente é implícito ser instantâneo, com apenas a interação nominal ocorrendo com a variável. No entanto, isso geralmente também se aplica apenas a tipos simples, tipos de dados complexos (ie List), acho mais difícil encapsular adequadamente uma propriedade e geralmente uso outros métodos para interagir com eles, em oposição a um mutador e um acessador estritos.

rjzii
fonte
2

Chamar um getter deve exibir o mesmo comportamento que a leitura de um campo:

  • Deve ser barato recuperar o valor
  • Se você definir um valor com o setter e depois lê-lo com o getter, o valor deverá ser o mesmo
  • Obter o valor não deve ter efeitos colaterais
  • Não deve lançar uma exceção
Sjoerd
fonte
2
Eu não concordo completamente com isso. Concordo que não deve ter efeitos colaterais, mas acho perfeitamente adequado implementá-lo de uma maneira que o diferencie de um campo. Olhando para o .Net BCL, InvalidOperationException é amplamente usado quando se olha para getters. Além disso, consulte a resposta de MikeNakis sobre a ordem das operações.
Max
Concorde com todos os pontos, exceto o último. Certamente é possível que obter um valor possa envolver a realização de um cálculo ou outra operação que dependa de outros valores ou recursos que podem não ter sido definidos. Nesses casos, eu esperaria que o getter lançasse algum tipo de exceção.
TMN
1
@TMN: Na melhor das hipóteses, a classe deve ser organizada de forma que os getters não precisem executar operações capazes de gerar exceção. Minimizar os lugares que podem gerar exceções leva a menos surpresas inesperadas.
23911 hugomg
8
Vou discordar com o segundo ponto com um exemplo específico: foo.setAngle(361); bar = foo.getAngle(). barpoderia ser 361, mas também pode ser legitimamente 1se os ângulos estão vinculados a um intervalo.
zzzzBov
1
-1. (1) é barato neste exemplo - após o carregamento lento. (2) atualmente não há nenhuma "setter" no exemplo, mas se alguém adiciona um depois, e ele simplesmente conjuntos stuff, o getter irá devolver o mesmo valor. (3) O carregamento lento, como mostrado no exemplo, não produz efeitos colaterais "visíveis". (4) é discutível, talvez um ponto válido, uma vez que a introdução do "carregamento lento" posteriormente pode alterar o contrato anterior da API - mas é preciso examinar esse contrato para tomar uma decisão.
Doc Brown
2

Um getter que chama outras propriedades e métodos para calcular seu próprio valor também implica uma dependência. Por exemplo, se sua propriedade precisa ser capaz de se calcular e, ao fazer isso, exige que outro membro seja definido, você precisa se preocupar com referências nulas acidentais se sua propriedade for acessada no código de inicialização, onde todos os membros não estão necessariamente definidos.

Isso não significa 'nunca acessar outro membro que não seja o campo de apoio das propriedades no getter', apenas significa prestar atenção ao que você está implicando sobre qual é o estado exigido do objeto e se isso corresponde ao contexto que você espera esta propriedade para ser acessada.

No entanto, nos dois exemplos concretos que você deu, a razão pela qual eu escolheria um sobre o outro é totalmente diferente. Seu getter é inicializado no primeiro acesso, por exemplo, Lazy Initialization . O segundo exemplo é assumido como inicializado em algum momento anterior, por exemplo, Inicialização Explícita .

Quando ocorre exatamente a inicialização, pode ou não ser importante.

Por exemplo, pode ser muito lento e precisa ser feito durante uma etapa de carregamento em que o usuário espera um atraso, em vez de apresentar um desempenho inesperado quando o usuário aciona o acesso pela primeira vez (ou seja, cliques com o botão direito do mouse, menu de contexto aparece, usuário já clicou com o botão direito novamente).

Além disso, às vezes há um ponto óbvio na execução em que tudo o que pode afetar / sujar o valor da propriedade em cache ocorre. Você pode até verificar se nenhuma das dependências muda e lança exceções posteriormente. Nessa situação, faz sentido também armazenar em cache o valor nesse ponto, mesmo que não seja particularmente caro calcular, apenas para evitar tornar a execução do código mais complexa e mais difícil de seguir mentalmente.

Dito isto, a Lazy Initialization faz muito sentido em muitas outras situações. Assim, como muitas vezes acontece na programação, é difícil resumir-se a uma regra, mas tudo se resume ao código concreto.

James
fonte
0

Basta fazê-lo como o @MikeNakis disse ... Se você acabou de receber as coisas, tudo bem ... Se você fizer outra coisa, crie uma nova função que faça o trabalho e torne-a pública.

Se sua propriedade / função está fazendo apenas o que o nome diz, não resta muito espaço para complicações. A coesão é a chave IMO

Ivan Crojach Karačić
fonte
1
Tenha cuidado com isso, você pode acabar expondo muito do seu estado interno. Você não quer acabar com muitos métodos vazios loadFoo()ou preloadDummyReferences()ou createDefaultValuesForUninitializedFields()apenas porque a implementação inicial da sua classe precisava deles.
TMN
Claro ... Eu estava apenas dizendo que se você fizer o que o nome diz que não deve haver muitos problemas ... mas o que você diz é absolutamente verdadeiro ... #
227 Ivan Crojach Karačić
0

Pessoalmente, eu exporia o requisito de Stuff por meio de um parâmetro no construtor e permitiria que qualquer classe que instanciava o material fizesse o trabalho de descobrir de onde deveria vir. Se o material for nulo, ele deve retornar nulo. Prefiro não tentar soluções inteligentes como o original do OP, porque é uma maneira fácil de ocultar bugs profundamente na sua implementação, onde não é óbvio o que pode estar errado quando algo quebra.

EricBoersma
fonte
0

Existem questões mais importantes do que apenas "adequação" aqui e você deve basear sua decisão nessas questões . Principalmente, a grande decisão aqui é se você deseja permitir que as pessoas ignorem o cache ou não.

  1. Primeiro, pense se existe uma maneira de reorganizar seu código para que todas as chamadas de carga e gerenciamento de cache necessários sejam feitos no construtor / inicializador. Se isso for possível, você pode criar uma classe cuja invariante permite que você faça o getter simples da parte 2 com a segurança do getter complexo da parte 1. (Um cenário em que todos saem ganhando)

  2. Se você não pode criar essa classe, decida se possui uma troca e precisa decidir se deseja permitir que o consumidor ignore o código de verificação de cache ou não.

    1. Se for importante que o consumidor nunca pule a verificação de cache e você não se importe com as penalidades de desempenho, junte a verificação dentro do getter e torne impossível que o consumidor faça a coisa errada.

    2. Se não houver problema em ignorar a verificação de cache ou for muito importante que você tenha desempenho garantido de O (1) no getter, use chamadas separadas.

Como você já deve ter notado, não sou um grande fã da filosofia "código limpo", "divida tudo em pequenas funções". Se você tiver várias funções ortogonais que podem ser chamadas em qualquer ordem, dividi-las fornecerá um poder mais expressivo a um custo baixo. No entanto, se suas funções tiverem dependências de ordem (ou forem realmente úteis em uma ordem específica), dividi-las apenas aumentará o número de maneiras pelas quais você pode fazer coisas erradas, além de adicionar poucos benefícios.

hugomg
fonte
-1, os construtores devem construir, não inicializar. Colocar a lógica do banco de dados em um construtor torna essa classe completamente não testável e, se você tiver mais do que alguns deles, o tempo de inicialização do aplicativo se tornará imenso. E isso é apenas para iniciantes.
Domenic
@ Domínio: Este é um problema semântico e dependente do idioma. O ponto em que um objeto está apto a ser usado e fornece os invariantes apropriados após e somente depois de serem totalmente construídos.
hugomg 23/12
0

Na minha opinião, Getters não deve ter muita lógica neles. Eles não devem ter efeitos colaterais e você nunca deve receber uma exceção deles. A menos, claro, você sabe o que está fazendo. A maioria dos meus getters não tem lógica neles e apenas vai para um campo. Mas a exceção notável a isso foi com uma API pública que precisava ser o mais simples possível de usar. Então, eu tinha um getter que falharia se outro getter não tivesse sido chamado. A solução? Uma linha de código como var throwaway=MyGetter;no getter que dependia dela. Não tenho orgulho disso, mas ainda não vejo uma maneira mais limpa de fazê-lo

Earlz
fonte
0

Parece uma leitura do cache com carregamento lento. Como outros observaram, a verificação e o carregamento podem pertencer a outros métodos. O carregamento pode precisar ser sincronizado para que você não receba vinte threads ao mesmo tempo.

Pode ser apropriado usar um nome getCachedStuff()para o getter, pois não terá um tempo de execução consistente.

Dependendo de como a cacheInvalid()rotina funciona, a verificação de nulo pode não ser necessária. Eu não esperaria que o cache fosse válido, a menos que stufftivesse sido preenchido no banco de dados.

BillThor
fonte
0

A principal lógica que eu esperaria ver nos getters que retornam uma lista é lógica para garantir que a lista não seja modificável. Tal como está, seu exemplo pode quebrar o encapsulamento.

algo como:

public List<Stuff> getStuff()
{
    return Collections.unmodifiableList(stuff);
}

quanto ao armazenamento em cache no getter, acho que isso seria bom, mas posso ficar tentado a mudar a lógica do cache se a criação do cache demorar um tempo significativo. isto depende.

jk.
fonte
0

Dependendo do caso de uso exato, eu gosto de separar meu cache em uma classe separada. Faça uma StuffGetterinterface, implemente uma StuffComputerque faça os cálculos e envolva-a em um objeto de StuffCacher, responsável por acessar o cache ou encaminhar chamadas para o StuffComputerque ele envolve.

interface StuffGetter {
     public List<Stuff> getStuff();
}

class StuffComputer implements StuffGetter {
     public List<Stuff> getStuff() {
         getStuffFromDatabase()
     }
}

class StuffCacher implements StuffGetter {
     private stuffComputer; // DI this
     private Cache<List<Stuff>> cache = new Cache<>();

     public List<Stuff> getStuff() {
         if cache.hasStuff() {
             return cache.getStuff();
         }

         List<Stuffs> stuffs = stuffComputer.getStuff();
         cache.store(stuffs);
         return stuffs;
     }
}

Esse design permite adicionar facilmente cache, remover cache, alterar a lógica de derivação subjacente (por exemplo, acessar um banco de dados versus retornar dados simulados) etc. É um pouco prolixo, mas vale a pena em projetos suficientemente avançados.

Alexander
fonte
-1

IMHO é muito simples se você usar um design por contrato. Decida o que o getter deve fornecer e apenas codifique de acordo (código simples ou alguma lógica complexa que possa estar envolvida ou delegada em algum lugar).

Kemoda
fonte
+1: eu concordo com você! Se o objeto tiver apenas o objetivo de armazenar alguns dados, os getters deverão retornar apenas o conteúdo atual do objeto. Nesse caso, é responsabilidade de algum outro objeto carregar os dados. Se o contrato indicar que o objeto é o proxy de um registro de banco de dados, o getter deverá buscar os dados rapidamente. Pode ficar ainda mais complicado se os dados foram carregados, mas não estão atualizados: o objeto deve ser notificado sobre alterações no banco de dados? Eu acho que não há uma resposta única para essa pergunta.
Giorgio