¿Es un olor a código si está creando un objeto con frecuencia solo para llamar a un método?

14

Heredé una base de código donde hay mucho código que va más o menos así:

SomeDataAdapter sda = new SomeDataAdapter();
sda.UpdateData(DataTable updateData);

Y luego sda nunca se usa de nuevo.

¿Es ese un olor a código que indica que esos métodos deberían ser métodos de clase estática en su lugar?

Shane Wealti
fuente
¿Estas clases implementan alguna interfaz?
Reactgular
77
El día después de refactorizar los métodos estáticos será el día en que desee ejecutar pruebas unitarias con una versión simulada de su adaptador de datos.
Mike
22
@ Mike: ¿Por qué necesitarías burlarte de un método estático? Me estoy cansando mucho de este argumento falaz de que los métodos estáticos no son comprobables. Si sus métodos estáticos mantienen el estado o crean efectos secundarios, ese es su culpa, no la culpa de su metodología de prueba.
Robert Harvey
99
En mi opinión, es un olor a lenguaje que muestra la debilidad del análisis de "todo debe ser una clase".
Ben
77
@RobertHarvey: es posible que deba burlarse de un método estático por la misma razón que se burla de cualquier otro método: es demasiado costoso llamar durante la prueba de la unidad.
Kevin Cline

Respuestas:

1

Lo consideraría un olor a arquitectura en que UpdateData probablemente debería pertenecer a una clase de 'servicio'.

Donde los datos son una manzana. Donde AppleAdapter es clase de servicio / inteligencia de negocios. Donde AppleService es una referencia Singleton a un AppleAdapter que existe fuera del método actual.

private static volatile AppleAdapter _appleService = null;
private static object _appleServiceLock = new object();
private AppleAdapter AppleService
{
    get
    {
        if (_appleService == null)
        {
            lock (_appleServiceLock)
            {
                if (_appleService == null)
                    _appleService = new AppleAdapter();
            }
        }
        return _appleService;
    }
}

public SomeAppleRelatedMethod(Apple apple)
{
    AppleService.UpdateData(apple);
}

No creo que lo que está haciendo esté necesariamente mal, pero si SomeDataAdapter realmente representa algún tipo de servicio empresarial sin estado, entonces un singleton sería la mejor práctica para ello. ¡Espero que ayude! El ejemplo proporcionado es una forma elegante de garantizar que no haya contención del _appleService si es que ambos o dos subprocesos son nulos y se accede a ellos exactamente al mismo tiempo.

¿Sabes que? Si SomeDataAdapter es un ADO IDbDataAdapter (que casi seguro es), ¡ignore toda esta respuesta!

:PAG

No tengo permiso para agregar un comentario a la pregunta original, pero si pudiera especificar dónde existe este código.

Si este código representa una implementación personalizada de un IDbDataAdapter, y UpdateData está creando un IDbConnection, IDbCommand y conecta todo detrás de escena, entonces no consideraría que un código huele mal porque ahora estamos hablando de streams y otros cosas que deben eliminarse cuando hayamos terminado de usarlas.

Andrew Hoffman
fuente
12
¡Ayuda! Me estoy ahogando en los patrones de software.
Robert Harvey
44
... o inyectado como una dependencia. ;)
Rob
12
¡Jugar con singletons y bloqueo doblemente verificado para obtener algo equivalente a una función / procedimiento simple es un olor de código mucho más grande para mí!
Ben
3

Creo que este problema es aún más profundo que eso. Incluso si lo refactoriza en un método estático, obtiene un código donde llama al método estático único en todo el lugar. Lo que en mi opinión es código de olor en sí mismo. Podría indicar que te falta alguna abstracción importante. Tal vez desee crear un código que haga un procesamiento previo y posterior mientras le permite cambiar lo que sucede en el medio. Ese procesamiento previo y posterior contendrá las llamadas comunes, mientras que el intermedio dependerá de la implementación concreta.

Eufórico
fuente
Estoy de acuerdo en que hay un problema mucho más profundo. Estoy tratando de mejorarlo gradualmente sin un rediseño arquitectónico completo, pero parece que lo que estaba pensando no es una buena manera de hacerlo.
Shane Wealti
1

Mas o menos. Por lo general, significa que desea pasar una función y su idioma de elección no le permitirá. Es un poco torpe y poco elegante, pero si estás atrapado en un idioma sin funciones de primera clase, no hay mucho que puedas hacer.

Si ninguno de esos objetos se pasa como argumento a otras funciones, podría convertirlos en métodos estáticos y nada cambiaría.

Doval
fuente
También podría significar que desea que el método estático devuelva una versión modificada (o una copia modificada) de un objeto que le está pasando.
Robert Harvey
2
"si está atrapado en un lenguaje sin funciones de primera clase": no hay diferencia entre un objeto con un solo método y una función de primera clase (o cierre de primera clase): son solo dos vistas diferentes de la misma cosa.
Giorgio
44
@ Jorge Teóricamente, no. En la práctica, si su idioma al menos no tiene un azúcar de sintaxis, es la diferencia entre fn x => x + 1y / new Function<Integer, Integer>() { public Integer eval(Integer x) { return x + 1; }};o limitarse a unas pocas funciones codificadas y de poca utilidad.
Doval
¿Por qué no podría fn x => x + 1ser azúcar sintáctico new Function<Integer, Integer>() { public Integer eval(Integer x) { return x + 1; }}? Ok, tal vez te refieres a si uno está atrapado en un idioma sin este tipo de azúcar sintáctica.
Giorgio
@Giorgio Tendrás que preguntarle a Oracle. (De acuerdo, eso finalmente está cambiando con Java 8). Tenga en cuenta que necesitaría más azúcar de sintaxis que solo eso para ser realmente útil; también querría poder pasar métodos pre declarados por nombre. Curry también sería útil. En algún momento, debe preguntarse si vale la pena tener todos esos hacks en lugar de tratar las funciones como primeros ciudadanos. Pero ahora me estoy saliendo del tema.
Doval
0

Si el estado de espera hace que una clase dada sea más utilitaria, funcional ... reutilizable, entonces no. Por alguna razón , me viene a la mente el patrón de diseño del comando ; esa razón ciertamente no es un deseo de ahogar a Robert Harvey.

radarbob
fuente
0

Definir "con frecuencia". ¿ Con qué frecuencia crea una instancia del objeto? Mucho, ¿probablemente no?

Es probable que haya problemas más importantes por los que preocuparse en su sistema. Si se vuelve estático, se comunicará más claramente al programador que el estado interno del objeto no está cambiando, excelente.

Sin embargo, si se está alterando el estado y tiene que comenzar a hacer que otras cosas sean estáticas para que la primera sea estática, entonces debe preguntar qué partes deben ser estáticas y cuáles no.

, es un olor a código, solo uno pequeño.

usuario1775944
fuente
0

Conviértalo en un método estático y siga adelante. No hay nada malo con los métodos estáticos. Una vez que surge un patrón de uso, puede preocuparse por abstraerlo.

davidk01
fuente
0

El olor aquí es que este es un código de procedimiento envuelto (mínimamente) en objetos.

Debe haber una razón por la que desea realizar esa operación en la tabla de datos, pero en lugar de modelar esa operación con otras operaciones relacionadas que comparten alguna semántica (es decir, tienen un significado común), el autor simplemente desarrolló un nuevo procedimiento dentro de un nuevo clase y lo llamó.

En un lenguaje funcional, así es como haría las cosas;) pero en el paradigma OO, querría modelar alguna abstracción que incluyera esa operación. Luego usaría técnicas OO para desarrollar ese modelo y proporcionar un conjunto de operaciones relacionadas que preserven algo de semántica.

Entonces, el olor principal aquí es que el autor está usando clases, probablemente porque el compilador lo requiere, sin organizar las operaciones en un modelo conceptual.

Por cierto, ten cuidado cuando veas que el programa se está modelando en lugar del espacio del problema . Es una señal de que el diseño podría beneficiarse de más pensamiento. En otras palabras, tenga cuidado con las clases, todas "xAdaptor" y "xHandler" y "xUtility", y generalmente están vinculadas a un modelo de dominio anémico . Significa que alguien solo agrupa el código por conveniencia, en lugar de modelar realmente los conceptos que desea implementar.

Robar
fuente