Nombre para este antipatrón? Campos como variables locales [cerrado]

En algún código que estoy revisando, veo cosas que son el equivalente moral de lo siguiente:

public class Foo
{
    private Bar bar;

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

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

El campo baraquí es lógicamente una variable local, ya que su valor nunca pretende persistir en las llamadas a métodos. Sin embargo, dado que muchos de los métodos Foonecesitan un objeto de tipo Bar, el autor del código original acaba de crear un campo de tipo Bar.

1. Esto obviamente es malo, ¿verdad?
2. ¿Hay un nombre para este antipatrón?

Esto obviamente es malo, ¿verdad?

Sí.

• Hace que los métodos no sean reentrantes, lo cual es un problema si se invocan en la misma instancia de forma recursiva o en un contexto de subprocesos múltiples.

• Significa que el estado de una llamada se filtra a otra llamada (si olvida reiniciar).

• Hace que el código sea difícil de entender porque debe verificar lo anterior para asegurarse de qué está haciendo realmente el código. (Contraste con el uso de variables locales).

• Hace que cada Fooinstancia sea más grande de lo necesario. (E imagine hacer esto para N variables ...)

¿Hay un nombre para este antipatrón?

En mi opinión, esto no merece ser llamado antipatrón. Es solo un mal hábito de codificación / un mal uso de construcciones Java / código basura. Es el tipo de cosas que puede ver en el código de un estudiante universitario cuando el estudiante ha estado omitiendo conferencias y / o no tiene aptitud para la programación. Si lo ve en el código de producción, es una señal de que necesita hacer muchas más revisiones de código ...

Para el registro, la forma correcta de escribir esto es:

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();
    }
}

Tenga en cuenta que también he arreglado el método y los nombres de las variables para que se ajusten a las reglas de estilo aceptadas para los identificadores Java.

Yo lo llamaría un gran alcance innecesario para una variable. Esta configuración también permite condiciones de carrera si varios subprocesos acceden al Método A y al Método B.

Este es un caso específico de un patrón general conocido como global doorknobbing. Al construir una casa, es tentador comprar solo un pomo y dejarlo tirado. Cuando alguien quiere usar una puerta o un armario, simplemente agarra ese pomo de la puerta global. Si los pomos de las puertas son caros, puede ser un buen diseño.

Desafortunadamente, cuando su amigo se acerca y el pomo de la puerta se usa simultáneamente en dos lugares diferentes, la realidad se rompe. Si el pomo de la puerta es tan caro que solo puede permitirse uno, entonces vale la pena construir un sistema que permita a las personas esperar su turno de manera segura.

En este caso, el pomo de la puerta es solo una referencia, por lo que es barato. Si el pomo de la puerta era un objeto real (y estaban reutilizando el objeto en lugar de solo la referencia), entonces podría ser lo suficientemente costoso como para ser un prudent global doorknob. Sin embargo, cuando es barato, se conoce como a cheapskate global doorknob.

Tu ejemplo es de la cheapskatevariedad.

La principal preocupación aquí sería la concurrencia: si el Foo está siendo utilizado por múltiples hilos, tiene un problema.

Más allá de eso, es una tontería: las variables locales manejan sus propios ciclos de vida perfectamente bien. Reemplazarlos con variables de instancia que necesitan ser anuladas cuando ya no son útiles es una invitación al error.

Es un caso específico de "alcance inadecuado", con un lado de "reutilización variable".

No es un antipatrón. Los antipatrones tienen alguna propiedad que hace que parezca una buena idea, lo que lleva a las personas a hacerlo a propósito; se planean como patrones y luego sale terriblemente mal.

También es lo que genera debates sobre si algo es un patrón, un antipatrón o un patrón comúnmente mal aplicado que todavía tiene usos en algunos lugares.

Esto está mal.

Para agregar un poco más.

Este código es supersticioso o, en el mejor de los casos, una práctica de culto de carga.

Una superstición es algo hecho sin una justificación clara. Puede estar relacionado con algo real, pero la conexión no es lógica.

Una práctica de culto de carga es aquella en la que intentas copiar algo que has aprendido de una fuente más informada, pero en realidad estás copiando los artefactos de superficie en lugar del proceso (llamado así por un culto en Papua Nueva Guinea que haría radios de control de aeronaves de bambú con la esperanza de hacer que regresen los aviones japoneses y estadounidenses de la Segunda Guerra Mundial).

En ambos casos, no hay ningún caso real por hacer.

Un antipatrón es un intento de mejora razonable, ya sea en el pequeño (esa ramificación adicional para tratar ese caso adicional que tiene que ser tratado, que conduce a un código de espagueti) o en el grande donde implementas un patrón muy deliberadamente que está desacreditado o debatido (muchos describirían los singletons como tales, con algunos excluyendo solo escritura, por ejemplo, objetos de registro o solo lectura, por ejemplo, objetos de configuración, y algunos condenarían incluso esos) o si está resolviendo el problema problema incorrecto (cuando se presentó .NET por primera vez, MS recomendó un patrón para lidiar con la eliminación cuando tenía campos no administrados y campos administrados desechables; de hecho, trata esa situación muy bien, pero el verdadero problema es que tiene ambos tipos de campo en la misma clase).

Como tal, un antipatrón es algo que una persona inteligente que conoce bien el idioma, el dominio del problema y las bibliotecas disponibles hará deliberadamente, que todavía tiene (o se argumenta que tiene) una desventaja que abruma la ventaja.

Dado que ninguno de nosotros comienza a conocer bien un idioma determinado, un dominio problemático y las bibliotecas disponibles, y dado que todos pueden perder algo a medida que pasan de una solución razonable a otra (por ejemplo, comenzar a almacenar algo en un campo para un buen uso, y luego intentar refactorizarlo pero no completar el trabajo, y terminarás con el código como en la pregunta), y dado que todos extrañamos las cosas de vez en cuando en el aprendizaje, todos hemos creado algún código supersticioso o culto a la carga en algún momento . Lo bueno es que en realidad son más claros de identificar y corregir que los antipatrones. Los antipatrones verdaderos no son posiblemente antipatrones, o tienen una cualidad atractiva, o al menos tienen alguna forma de atraer a uno incluso cuando se identifican como malos (demasiadas y muy pocas capas son indiscutiblemente malas, pero evitan una conduce a la otra).

(1) Yo diría que no es bueno.

Está realizando tareas de mantenimiento manuales que la pila puede hacer automáticamente con variables locales.

No hay beneficio de rendimiento ya que "nuevo" se llama invocación de cada método.

EDITAR: la huella de memoria de la clase es mayor porque el puntero de barra siempre ocupa memoria durante la vida útil de la clase. Si el puntero de la barra fuera local, solo usaría memoria durante la vida útil de la llamada al método. La memoria para el puntero es solo una gota en el océano, pero sigue siendo una gota innecesaria.

La barra es visible para otros métodos en la clase. Los métodos que no usan barra no deberían poder verlo.

Errores basados ​​en estado. En este caso particular, los errores de base de estado solo deberían ocurrir en subprocesos múltiples, ya que se llama "nuevo" cada vez.

(2) No sé si hay un nombre para él, ya que en realidad son varios problemas, no uno.
- limpieza manual cuando la automática está disponible
- estado innecesario - estado
que dura más de su vida útil - la
visibilidad o el alcance es demasiado alto

Yo lo llamaría el "Xenófobo errante". Quiere quedarse solo, pero termina sin saber exactamente dónde o qué es. Por lo tanto, es algo malo como han dicho otros.

Yendo contra la corriente aquí, pero si bien no considero que el ejemplo dado sea un buen estilo de codificación como está en su forma actual, el patrón existe mientras se realizan las pruebas unitarias.

Esta forma bastante estándar de hacer pruebas unitarias

public class BarTester
{
    private Bar bar;

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

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

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

es solo una refactorización de este equivalente del código de 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;
    }
}

Nunca escribiría código como lo da el ejemplo de OP, grita refactorización, pero considero que el patrón no es inválido ni antipatrón .

Beck / Fowler hace referencia a este olor a código como Campo temporal en refactorización.

http://sourcemaking.com/refactoring/temporary-field

Editado para agregar más explicaciones a pedido de los moderadores: puede leer más sobre el olor del código en la URL de referencia anterior o en la copia impresa del libro. Dado que el OP estaba buscando el nombre de este olor a antipatrón / código en particular, pensé que sería útil citar una referencia no mencionada hasta ahora de una fuente establecida que tiene más de 10 años, aunque me doy cuenta de que llego tarde a la juego sobre esta pregunta, por lo que es poco probable que la respuesta sea votada o aceptada.

Si no es demasiado promocional personalmente, también hago referencia y explico este olor a código en particular en mi próximo curso de Pluralsight, Refactoring Fundamentals, que espero se publique en agosto de 2013.

A modo de agregar más valor, hablemos sobre cómo solucionar este problema en particular. Hay varias opciones Si el campo realmente solo se usa con un método, se puede degradar a una variable local. Sin embargo, si varios métodos están utilizando el campo para comunicarse (en lugar de parámetros), entonces este es el caso clásico descrito en Refactorización, y puede corregirse reemplazando el campo con parámetros, o si los métodos que funcionan con este código están estrechamente relacionados relacionado, la clase de extracto se puede usar para extraer los métodos y los campos en su propia clase, en la que el campo siempre está en un estado válido (quizás establecido durante la construcción) y representa el estado de esta nueva clase. Si va a la ruta de parámetros, pero hay demasiados valores para pasar, otra opción es introducir un nuevo objeto de parámetro,

Yo diría que, en el fondo, esto es realmente una falta de cohesión, y podría darle el nombre de "Falsa Cohesión". Yo acuñaría (o si lo obtengo de algún lado y lo olvido, "pedir prestado") este término porque la clase parece ser coherente en el sentido de que sus métodos parecen funcionar en un campo miembro. Sin embargo, en realidad no lo hacen, lo que significa que la cohesión aparente es realmente falsa.

En cuanto a si es malo o no, diría que claramente lo es, y creo que Stephen C hace un excelente trabajo al explicar por qué. Sin embargo, si merece ser llamado "antipatrón" o no, creo que cualquier otro paradigma de codificación podría funcionar con una etiqueta, ya que eso generalmente facilita la comunicación.

Además de los problemas ya mencionados, el uso de este enfoque en un entorno sin recolección de basura automática (por ejemplo, C ++) provocaría pérdidas de memoria, ya que los objetos temporales no se liberan después del uso. (Si bien esto puede parecer un poco exagerado, hay personas que simplemente cambiarían 'nulo' a 'NULO' y estarían felices de que el código se compile).

Esto me parece una aplicación incorrecta horrible del patrón Flyweight. Desafortunadamente, he conocido a algunos desarrolladores que tienen que usar la misma "cosa" varias veces en diferentes métodos y simplemente sacarla a un campo privado en un intento equivocado de ahorrar memoria o mejorar el rendimiento o alguna otra pseudooptimización extraña. En su opinión, están tratando de resolver un problema similar, ya que Flyweight está destinado a abordar solo que lo están haciendo en una situación en la que en realidad no es un problema que deba resolverse.

Me he encontrado con esto muchas veces, generalmente al actualizar el código previamente escrito por alguien que eligió VBA For Dummies como su primer título y luego escribió un sistema relativamente grande en el idioma en el que se habían topado.

Decir que es realmente una mala práctica de programación es correcto, pero podría haber sido el resultado de no tener recursos revisados ​​por Internet y por pares, como todos disfrutamos hoy, que podrían haber guiado al desarrollador original por un camino "más seguro".

Sin embargo, realmente no merece la etiqueta "antipatrón", pero ha sido bueno ver las sugerencias de cómo se podría recodificar el OP.