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

67

En algunos códigos 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 bar aquí es lógicamente una variable local, ya que su valor nunca tiene la intención de persistir en las llamadas a métodos. Sin embargo, dado que muchos de los métodos en Foo necesitan un objeto de tipo Bar , el autor del código original acaba de crear un campo de tipo Bar .

  1. Esto es obviamente malo, ¿verdad?
  2. ¿Hay un nombre para este antipatrón?
pregunta JSBձոգչ 31.08.2012 - 16:25

14 respuestas

85
  

Esto es obviamente malo, ¿verdad?

Sí.

  • Hace que los métodos no sean reentrantes, lo cual es un problema si se los llama 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 reinicializar).

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

  • Hace que cada instancia Foo sea más grande de lo que debe ser. (E imagina hacer esto para N variables ...)

  

¿Hay un nombre para este antipatrón?

OMI, esto no merece ser llamado antipatrón. Es solo un mal hábito de codificación / un mal uso de las construcciones de Java / código basura. Es el tipo de cosa que puede ver en el código de un estudiante universitario cuando el estudiante ha estado faltando a las clases y / o no tiene aptitudes 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();
    }
}

Observe que también he corregido los nombres de métodos y variables para que se ajusten a las reglas de estilo aceptadas para los identificadores de Java.

    
respondido por el Stephen C 31.08.2012 - 16:41
39

Yo lo llamaría 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.

    
respondido por el Th 0 mÄ s 31.08.2012 - 16:30
30

Este es un caso específico de un patrón general conocido como global doorknobbing . Al construir una casa, es tentador comprar solo un tirador y dejarlo tirado. Cuando alguien quiere usar una puerta o un armario, simplemente agarran esa perilla global. Si las perillas de las puertas son caras, 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 tirador de la puerta es tan caro que solo puedes pagar uno, vale la pena construir un sistema que permita a las personas esperar su turno para el tirador de la puerta.

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 reutilizaban el objeto en lugar de solo la referencia), entonces podría ser lo suficientemente caro como para ser un prudent global doorknob . Sin embargo, cuando es barato, se conoce como cheapskate global doorknob .

Su ejemplo es de la variedad cheapskate .

    
respondido por el Ned Twigg 31.08.2012 - 23:47
19

La principal preocupación aquí sería la concurrencia: si Foo está siendo utilizado por varios subprocesos, tiene un problema.

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

    
respondido por el Matthew Flynn 31.08.2012 - 16:33
15

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

    
respondido por el ikegami 31.08.2012 - 20:10
7

No es un anti-patrón. Los anti-patrones 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 se vuelven terriblemente mal.

También es lo que hace a los 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 es simplemente incorrecto.

Para agregar un poco más.

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

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

Una práctica de culto a la carga es aquella en la que intenta copiar algo que aprendió de una fuente con más conocimientos, pero en realidad está copiando los artefactos de la superficie en lugar del proceso (llamado así para un culto en Papua Nueva Guinea ¿Quién haría que los radios de control de aviones fuera del bambú con la esperanza de hacer que los aviones japoneses y estadounidenses de la Segunda Guerra Mundial regresen?

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

Un antipatrón es un intento de una mejora razonable, ya sea en la pequeña (esa ramificación adicional para tratar ese caso adicional con el que hay que lidiar, que conduce al código de espagueti) o en la grande donde usted deliberadamente implemente un patrón que sea desacreditado o debatido (muchos describirían los singletons como tales, algunos excluyendo solo escritura, por ejemplo, objetos de registro u objetos de configuración de solo lectura, y algunos condenarán incluso aquellos) o donde quiera Resolviendo el problema incorrecto (cuando se sacó .NET por primera vez, MS recomendó un patrón para manejar la eliminación cuando tenía campos no administrados y campos administrados desechables; de hecho, se ocupa de esa situación muy bien, pero el problema real es que usted tengo ambos tipos de campo en la misma clase).

Como tal, un anti-patró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 dice que tiene) un inconveniente que abruma el lado positivo. p>

Ya que ninguno de nosotros comienza a conocer bien un idioma, un dominio de problemas y bibliotecas disponibles, y dado que todos pueden perder algo al pasar de una solución razonable a otra (por ejemplo, comenzar a almacenar algo en un campo para un buen uso, y luego intente refactorizarlo pero no complete el trabajo, y terminará con un código como en la pregunta), y dado que todos extrañamos cosas de vez en cuando en el aprendizaje, todos hemos creado un código supersticioso o de culto a la carga. en algún momento. Lo bueno es que en realidad son más claros de identificar y corregir que los anti-patrones. Podría decirse que los verdaderos antipatrones no son antipatrones, o tienen una calidad atractiva, o al menos tienen alguna forma de atraerlos, incluso cuando se los identifica como malos (demasiadas y muy pocas capas son indiscutiblemente malas, pero evitan una lleva al otro).

    
respondido por el Jon Hanna 01.09.2012 - 00:38
3

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

Es un mantenimiento manual de la casa que la pila puede hacer automáticamente con las variables locales.

No hay beneficio en el 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 está ocupando memoria durante la vida útil de la clase. Si el puntero de barra era local, solo usaría la 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 la barra no deberían poder verlo.

Errores basados en estado. En este caso particular, los errores de la base de estado solo deberían ocurrir en los 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.
- El servicio de limpieza manual cuando automático está disponible
- estado innecesario
--estado que vive más tiempo que es de por vida
--visibilidad o alcance es demasiado alto

    
respondido por el mike30 31.08.2012 - 17:02
2

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

    
respondido por el World Engineer 31.08.2012 - 23:40
2

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

Esta forma bastante estándar de realizar 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 el código dado por el ejemplo de OP, grita refactorización, pero considero que el patrón no es inválido ni antipattern .

    
respondido por el Lieven Keersmaekers 03.09.2012 - 08:28
2

Este olor de código se conoce como Campo temporal en refactorización por Beck / Fowler.

enlace

Editado para agregar más explicaciones a petición de los moderadores: Puede leer más sobre el olor del código en la URL mencionada anteriormente, o en la copia impresa del libro. Como el OP estaba buscando el nombre para este particular olor a antipattern / código, 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 al juego en esta pregunta, por lo que es poco probable que la respuesta sea votada o aceptada.

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

Para agregar más valor, hablemos sobre cómo solucionar este problema en particular. Hay varias opciones. Si el campo es realmente solo utilizado por un método, entonces puede ser degradado a una variable local. Sin embargo, si varios métodos utilizan el campo para comunicarse (en lugar de los parámetros), este es el caso clásico descrito en Refactorización, y se puede corregir reemplazando el campo con parámetros, o si los métodos que funcionan con este código son muy similares. relacionado, la Clase de extracción se puede usar para extraer los métodos y los campos en su propia clase, en la que el campo está siempre en un estado válido (quizás establecido durante la construcción) y representa el estado de esta nueva clase. Si va por la ruta de parámetros, pero hay demasiados valores para pasar, otra opción es introducir un nuevo objeto de parámetro, que se puede pasar en lugar de todas las variables individuales.

    
respondido por el ssmith 03.07.2013 - 04:12
1

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

En cuanto a si es malo o no, diría que sí lo es, y creo que Stephen C hace un excelente trabajo explicando por qué. Sin embargo, ya sea que merezca llamarse "antipatrón" o no, creo que cualquier otro paradigma de codificación podría hacer con una etiqueta, ya que generalmente facilita la comunicación.

    
respondido por el Erik Dietrich 31.08.2012 - 23:40
1

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

    
respondido por el peterph 03.09.2012 - 14:33
1

Esto me parece una mala aplicación horrible del patrón de peso mosca. Desafortunadamente, conozco a algunos desarrolladores que tienen que usar la misma "cosa" varias veces con diferentes métodos y simplemente extraerlo en 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 el peso mosca está destinado a solucionar solo lo que están haciendo en una situación en la que no es realmente un problema que deba resolverse.

    
respondido por el Evicatos 03.07.2013 - 20:08
-1

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

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

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

    
respondido por el Lee James 06.09.2012 - 22:24

Lea otras preguntas en las etiquetas