Pasar un objeto a un método que lo cambia, ¿es un patrón común (anti)?

Estoy leyendo sobre olores comunes de códigos en el libro Refactoring de Martin Fowler . En ese contexto, me preguntaba acerca de un patrón que estoy viendo en una base de código, y si uno podría considerarlo objetivamente un antipatrón.

El patrón es uno donde se pasa un objeto como argumento a uno o más métodos, todos los cuales cambian el estado del objeto, pero ninguno de los cuales devuelve el objeto. Por lo tanto, depende del paso por naturaleza de referencia de (en este caso) C # /. NET.

var something = new Thing();
// ...
Foo(something);
int result = Bar(something, 42);
Baz(something);

Encuentro que (especialmente cuando los métodos no se nombran adecuadamente) necesito investigar dichos métodos para comprender si el estado del objeto ha cambiado. Hace que la comprensión del código sea más compleja, ya que necesito rastrear múltiples niveles de la pila de llamadas.

Me gustaría proponer mejorar dicho código para devolver otro objeto (clonado) con el nuevo estado, o cualquier cosa que sea necesaria para cambiar el objeto en el sitio de la llamada.

var something1 =  new Thing();
// ...

// Let's return a new instance of Thing
var something2 = Foo(something1);

// Let's use out param to 'return' other info about the operation
int result;
var something3 = Bar(something2, out result);

// If necessary, let's capture and make explicit complex changes
var changes = Baz(something3)
something3.Apply(changes);

Para mí, parece que el primer patrón se elige en los supuestos

• que es menos trabajo o requiere menos líneas de código
• que nos permite cambiar el objeto y devolver otra información
• que es más eficiente ya que tenemos menos casos de Thing.

Ilustra una alternativa, pero para proponerla, uno necesita tener argumentos en contra de la solución original. ¿Qué argumentos, si hay alguno, se pueden hacer para argumentar que la solución original es un antipatrón?

¿Y qué, si hay algo, está mal con mi solución alternativa?

Sí, la solución original es un antipatrón por las razones que usted describe: hace que sea difícil razonar sobre lo que está sucediendo, el objeto no es responsable de su propio estado / implementación (ruptura de la encapsulación). También agregaría que todos esos cambios de estado son contratos implícitos del método, lo que hace que ese método sea frágil ante los requisitos cambiantes.

Dicho esto, su solución tiene algunas de sus desventajas, la más obvia de las cuales es que la clonación de objetos no es excelente. Puede ser lento para objetos grandes. Puede conducir a errores donde otras partes del código se aferran a las referencias antiguas (lo que probablemente sea el caso en la base de código que describe). Hacer esos objetos explícitamente inmutables resuelve al menos algunos de estos problemas, pero es un cambio más drástico.

A menos que los objetos sean pequeños y algo transitorios (lo que los hace buenos candidatos para la inmutabilidad), me inclinaría simplemente a trasladar más parte de la transición de estado a los objetos mismos. Eso le permite ocultar los detalles de implementación de estas transiciones y establecer requisitos más estrictos sobre quién / qué / dónde ocurren esas transiciones de estado.

cuando los métodos no se nombran adecuadamente

En realidad, ese es el verdadero código de olor. Si tiene un objeto mutable, proporciona métodos para cambiar su estado. Si tiene una llamada a dicho método incrustado en una tarea de algunas declaraciones más, está bien refactorizar esa tarea a un método propio, lo que le deja exactamente en la situación descrita. Pero si no tiene nombres de métodos como Fooy Bar, pero métodos que dejan en claro que cambian el objeto, no veo un problema aquí. Pensar en

void AddMessageToLog(Logger logger, string msg)
{
    //...
}

o

void StripInvalidCharsFromName(Person p)
{
// ...
}

o

void AddValueToRepo(Repository repo,int val)
{
// ...
}

o

void TransferMoneyBetweenAccounts(Account source, Account destination, decimal amount)
{
// ...
}

o algo así: no veo ninguna razón aquí para devolver un objeto clonado para esos métodos, y tampoco hay ninguna razón para analizar su implementación para comprender que cambiarán el estado del objeto pasado.

Si no desea efectos secundarios, haga que sus objetos sean inmutables, impondrá métodos como los anteriores para devolver un objeto modificado (clonado) sin cambiar el original.

Sí, consulte http://codebetter.com/matthewpodwysocki/2008/04/30/side-effecting-functions-are-code-smells/ para uno de los muchos ejemplos de personas que señalan que los efectos secundarios inesperados son malos.

En general, el principio fundamental es que el software está construido en capas, y cada capa debe presentar la abstracción más limpia posible a la siguiente. Y una abstracción limpia es aquella en la que debes tener en cuenta lo menos posible para usarla. Eso se llama modularidad y se aplica a todo, desde funciones individuales hasta protocolos en red.

En primer lugar, esto no depende de la "naturaleza de paso por referencia de", depende de que los objetos sean tipos de referencia mutables. En lenguajes no funcionales, casi siempre será así.

En segundo lugar, si esto es un problema o no, depende tanto del objeto como de cuán estrechamente unidos estén los cambios en los diferentes procedimientos: si no realiza un cambio en Foo y eso hace que Bar se bloquee, entonces es un problema. No necesariamente es un olor a código, pero es un problema con Foo o Bar o Something (probablemente Bar, ya que debería estar verificando su entrada, pero podría ser algo que se está poniendo en un estado no válido que debería evitar).

No diría que se eleva al nivel de un antipatrón, sino algo a tener en cuenta.

Yo diría que hay poca diferencia entre A.Do(Something)modificar somethingy something.Do()modificar something. En cualquiera caso, debe quedar claro a partir del nombre del método invocado que somethingse modificará. Si el nombre del método no lo aclara, independientemente de si somethinges un parámetro thiso parte del entorno, no debe modificarse.

Creo que está bien cambiar el estado del objeto en algunos escenarios. Por ejemplo, tengo una lista de usuarios y quiero aplicar diferentes filtros a la lista antes de devolverla al cliente.

var users = Dependency.Resolve<IGetUsersQuery>().GetAll();

var excludeAdminUsersFilter = new ExcludeAdminUsersFilter();
var filterByAnotherCriteria = new AnotherCriteriaFilter();

excludeAdminUsersFilter.Apply(users);
filterByAnotherCriteria.Apply(users); 

Y sí, puede hacer que esto sea bonito moviendo el filtrado a otro método, por lo que terminará con algo como:

var users = Dependency.Resolve<IGetUsersQuery>().GetAll();
Filter(users);

Donde Filter(users)se ejecutarían los filtros anteriores.

No recuerdo exactamente dónde me encontré con esto antes, pero creo que se lo denominó tubería de filtrado.

No estoy seguro de si la nueva solución propuesta (de copiar objetos) es un patrón. El problema, como has señalado, es la mala nomenclatura de las funciones.

Digamos que escribo una operación matemática compleja como una función f () . Yo documento que f () es una función que asigna NXNa N, y el algoritmo detrás de él. Si la función se nombra de manera inapropiada, y no está documentada, y no tiene casos de prueba que la acompañen, deberá comprender el código, en cuyo caso el código no sirve para nada.

Sobre su solución, algunas observaciones:

• Las aplicaciones están diseñadas desde diferentes aspectos: cuando un objeto se usa solo para mantener valores, o se pasa a través de los límites de los componentes, es aconsejable cambiar las entrañas del objeto externamente en lugar de llenarlo con detalles sobre cómo cambiar.
• La clonación de objetos conduce a requisitos de memoria hinchada y, en muchos casos, a la existencia de objetos equivalentes en estados incompatibles (se Xconvirtió Ydespués f(), pero en Xrealidad es Y) y posiblemente inconsistencia temporal.

El problema que intenta abordar es válido; sin embargo, incluso con una enorme ingeniería excesiva realizada, el problema se evita, no se resuelve.