Nome para este antipadrão? Campos como variáveis locais [fechadas]

This is obviously bad, right?

Sim.

• Torna os métodos não reentrantes, o que é um problema se eles forem chamados na mesma instância de forma recursiva ou em um contexto multi-threaded.

• Isso significa que o estado de uma chamada vaza para outra chamada (se você se esquecer de reinicializar).

• Isso torna o código difícil de entender, porque você precisa verificar o que está acima para ter certeza do que o código está realmente fazendo. (Contraste com o uso de variáveis locais.)

• Isso torna cada instância Foo maior do que precisa ser. (E imagine fazer isso para as variáveis N ...)

Is there a name for this antipattern?

IMO, isso não merece ser chamado de antipadrão. É apenas um mau hábito de codificação / um uso indevido de código Java construct / crap. É o tipo de coisa que você pode ver em um código de graduação quando o aluno tem pulado palestras e / ou não tem aptidão para programação. Se você vê isso no código de produção, é um sinal de que você precisa fazer muito mais revisões de código ...

Para o registro, a maneira correta de escrever isso é:

public class Foo
{
    public methodA()
    {
        Bar bar = new Bar();  // Use a >>local<< variable!!
        bar.a();
    }

    // Or more concisely (in this case) ...
    public methodB()
    {
        new Bar().b();
    }
}

Observe que também consertei os nomes dos métodos e das variáveis para estar em conformidade com as regras de estilo aceitas para identificadores Java.

Eu diria que é um escopo grande e desnecessário para uma variável. Essa configuração também permite condições de corrida se vários segmentos acessarem MethodA e MethodB.

Este é um caso específico de um padrão geral conhecido como global doorknobbing . Ao construir uma casa, é tentador comprar apenas uma maçaneta e deixá-la por aí. Quando alguém quer usar uma porta ou armário, eles simplesmente pegam aquela maçaneta global. Se as maçanetas são caras, pode ser um bom design.

Infelizmente, quando seu amigo se aproxima e a maçaneta é usada simultaneamente em dois lugares diferentes, a realidade se despedaça. Se a maçaneta da porta é tão cara que você só pode pagar uma, então vale a pena construir um sistema que permita que as pessoas esperem a vez da maçaneta.

Neste caso, a maçaneta é apenas uma referência, por isso é barato. Se a maçaneta era um objeto real (e eles estavam reutilizando o objeto em vez de apenas a referência), talvez fosse caro o suficiente para ser um prudent global doorknob . No entanto, quando é barato, é conhecido como cheapskate global doorknob .

Seu exemplo é da variedade cheapskate .

A principal preocupação aqui seria a simultaneidade - Se o Foo estiver sendo usado por vários segmentos, você tem um problema.

Além disso, é apenas bobagem - variáveis locais gerenciam seus próprios ciclos de vida perfeitamente bem. Substituí-los por variáveis de instância que precisam ser anuladas quando não são mais úteis é um convite ao erro.

É um caso específico de "escopo impróprio", com um lado de "reutilização variável".

Não é um anti-padrão. Os antipadrões têm alguma propriedade que faz parecer uma boa ideia, o que leva as pessoas a fazer isso de propósito; eles são planejados como padrões e então tudo fica terrivelmente errado.

É também o que faz debates sobre se algo é um padrão, um antipadrão ou um padrão comumente mal aplicado que ainda tem usos em alguns lugares.

Isso está errado.

Para adicionar um pouco mais.

Este código é supersticioso, ou, na melhor das hipóteses, uma prática de culto de carga.

Uma superstição é algo feito sem uma justificativa clara. Pode estar relacionado a algo real, mas a conexão não é lógica.

Em ambos os casos, não há nenhum caso real a ser feito.

Um antipadrão é uma tentativa de uma melhora razoável, seja na pequena (aquela ramificação extra para lidar com aquele caso extra que precisa ser resolvido, que leva ao código do espaguete) ou no grande onde você deliberadamente implementar um padrão que seja desacreditado ou debatido (muitos descreveriam os singletons como tal, com alguns excluindo somente a gravação - por exemplo, registrando objetos ou objetos somente para leitura, por exemplo - e alguns condenariam até mesmo aqueles) ou então onde você Resolvendo o problema errado (quando o .NET foi lançado pela primeira vez, a MS recomendou um padrão para lidar com o descarte quando você tinha campos não gerenciados e campos gerenciados descartáveis - ele realmente lida com essa situação muito bem, mas o problema real é que você temos os dois tipos de campo na mesma classe).

Como tal, um antipadrão é algo que uma pessoa inteligente que conhece bem o idioma, o domínio do problema e as bibliotecas disponíveis fará deliberadamente, o que ainda tem (ou é argumentado ter) um lado negativo que supera o lado positivo.

Como nenhum de nós começa conhecendo bem uma determinada linguagem, domínio do problema e bibliotecas disponíveis, e já que todos podem perder algo ao passar de uma solução razoável para outra (por exemplo, começar a armazenar algo em um campo para um bom uso e tente refatorar, mas não completar o trabalho, e você vai acabar com o código como na pergunta), e como todos nós perdemos as coisas de vez em quando no aprendizado, todos criamos algum código supersticioso ou de culto à carga. em algum ponto. O bom é que eles são realmente mais claros para identificar e corrigir do que os antipadrões. Os antipadrões verdadeiros podem ser, sem dúvida, antipadrões, ou ter alguma qualidade atraente, ou pelo menos ter alguma forma de atraí-los, mesmo quando identificados como ruins (muitas e poucas camadas são ambas incontrolavelmente ruins, mas evitando uma leva ao outro).

(1) Eu diria que não é bom.

Ele está realizando manutenção manual manual que a pilha pode fazer automaticamente com variáveis locais.

Não há benefício de desempenho, já que "new" é chamado de chamada de cada método.

EDIT: A pegada de memória da classe é maior porque o ponteiro da barra está sempre ocupando a memória durante a vida útil da classe. Se o ponteiro de barra fosse local, ele usaria apenas memória para o tempo de vida da chamada do método. A memória do ponteiro é apenas uma gota no oceano, mas ainda é uma queda desnecessária.

A barra é visível para outros métodos na classe. Métodos que não usam barra não devem ser capazes de ver.

Erros com base em estado. Nesse caso específico, erros de base de estado só devem ocorrer em multiencadeamento, já que "new" é chamado a cada vez.

(2) Eu não sei se há um nome para ele, já que na verdade são vários problemas, não um.
serviço de limpeza manual quando automático está disponível
Estado desnecessário
estado que vive mais que é vida. --visibilidade ou escopo é muito alto

Eu chamaria isso de "Xenofobia Errante". Ele quer ficar sozinho, mas acaba não sabendo onde ou o que é. Assim, é uma coisa ruim, como outros afirmaram.

Indo contra a questão, mas embora eu não considere o exemplo dado como sendo um bom estilo de codificação como está em sua forma atual, o padrão existe ao fazer testes unitários.

Esta forma bastante padronizada de fazer testes unitários

public class BarTester
{
    private Bar bar;

    public Setup() { bar = new Bar(); }
    public Teardown() { bar = null; }

    public TestMethodA()
    {
        bar.A();        
    }

    public TestMethodB()
    {
        bar.B();
    }
}

é apenas uma refatoração desse equivalente do código do OP

public class BarTester
{
    private Bar bar;

    public TestMethodA()
    {
        bar = new Bar();
        bar.A();
        bar = null;
    }

    public TestMethodB()
    {
        bar = new Bar();
        bar.B();
        bar = null;
    }
}

Eu nunca escreveria código como dado pelo exemplo do OP, ele grita refatoração, mas eu considero que o padrão não é inválido nem um antipadrão .

Este cheiro de código é referido como Campo Temporário na Refatoração por Beck / Fowler.

link

Editado para adicionar mais explicações por solicitação dos moderadores: Você pode ler mais sobre o cheiro do código no URL mencionado acima ou na cópia impressa do livro. Como o OP estava procurando o nome para esse cheiro específico de antipadrão / código, achei que citar uma referência até então não mencionada a ele de uma fonte estabelecida com mais de 10 anos seria útil, embora eu tenha plena consciência de que estou atrasado para o jogo sobre esta questão, por isso é improvável que a resposta seja votada ou aceite.

Se não for muito promocional, também faço referência e explico esse cheiro específico de código no meu próximo curso da Pluralsight, Refactoring Fundamentals, que espero que seja publicado em agosto de 2013.

Para adicionar mais valor, vamos falar sobre como corrigir esse problema específico. Existem várias opções. Se o campo for realmente usado apenas por um método, ele poderá ser rebaixado para uma variável local. No entanto, se vários métodos estiverem usando o campo para se comunicar (em vez de parâmetros), esse é o caso clássico descrito em Refatoração e pode ser corrigido substituindo o campo por parâmetros ou se os métodos que trabalham com esse código estiverem intimamente relacionados Relacionado, Extrair Classe pode ser usado para puxar os métodos e os campos para fora em sua própria classe, em que o campo está sempre em um estado válido (talvez definido durante a construção) e representa o estado dessa nova classe. Se for a rota do parâmetro, mas houver muitos valores para passar, outra opção é introduzir um novo Objeto de Parâmetro, que pode ser passado em vez de todas as variáveis individuais.

Eu diria que quando você chegar a isso, isso é realmente uma falta de coesão, e eu posso dar o nome de "Coesão Falsa". Eu cunhei (ou se estou conseguindo isso de algum lugar e esquecendo-o, "peço emprestado") este termo porque a classe parece ser coesa, pois seus métodos parecem operar em um campo de membro. No entanto, na realidade eles não, significando que a coesão aparente é realmente falsa.

Quanto a ser ruim ou não, eu diria que é claramente, e acho que Stephen C faz um excelente trabalho explicando por quê. No entanto, se merece ser chamado de "anti-padrão" ou não, eu acho que isso e qualquer outro paradigma de codificação poderia fazer com um rótulo, uma vez que geralmente facilita a comunicação.

Além dos problemas já mencionados, o uso dessa abordagem no ambiente sem coleta de lixo automática (por exemplo, C ++) levaria a vazamentos de memória, pois os objetos temporários não são liberados após o uso. (Embora isso possa parecer um pouco improvável, existem pessoas por aí que apenas mudam 'null' para 'NULL' e ficam felizes que o código compila.)

Isso me parece uma péssima aplicação do padrão Flyweight. Eu infelizmente conheço alguns desenvolvedores que têm que usar a mesma "coisa" várias vezes em métodos diferentes e simplesmente puxá-lo para um campo privado em uma tentativa equivocada de economizar memória ou melhorar o desempenho ou alguma outra pseudo-otimização estranha. Em sua mente, eles estão tentando resolver um problema semelhante ao que o Flyweight pretende abordar apenas quando estão fazendo isso em uma situação em que não é realmente um problema que precisa ser resolvido.

Eu já encontrei isso muitas vezes, geralmente ao atualizar um código previamente escrito por alguém que pegou o VBA For Dummies como seu primeiro título e passou a escrever um sistema relativamente grande em qualquer idioma em que tivesse tropeçado.

Dizer que é uma prática ruim de programação está certo, mas pode ter sido o resultado de não ter internet & peer revisou recursos como todos nós desfrutamos hoje que poderia ter guiado o desenvolvedor original ao longo de um caminho "mais seguro".

Na verdade, ele não merece a tag "antipattern" - mas foi bom ver as sugestões de como o OP poderia ser recodificado.