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

12

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

El patrón es uno en el que un objeto se pasa como un 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, se basa en la naturaleza de paso por 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 son nombrados apropiadamente) necesito mirar tales métodos para entender si el estado del objeto ha cambiado. Hace que la comprensión del código sea más compleja, ya que necesito hacer un seguimiento de múltiples niveles de la pila de llamadas.

Me gustaría proponer que se mejore el 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);

A mi parecer, el primer patrón se elige sobre las suposiciones

  • 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 instancias de Thing .

Ilustro una alternativa, pero para proponerla, es necesario tener argumentos en contra de la solución original. ¿Qué argumentos, si los hay, se pueden presentar para demostrar que la solución original es un antipatrón?

¿Y qué hay de malo en mi solución alternativa?     

pregunta Michiel van Oosterhout 19.08.2013 - 18:22

7 respuestas

9

Sí, la solución original es un antipatrón por las razones que describe: hace que sea difícil razonar acerca de lo que está sucediendo, el objeto no es responsable de su propio estado / implementación (rompiendo 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 muy buena. Puede ser lento para objetos grandes. Puede llevar 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 usted describe). Hacer que esos objetos sean inmutables de forma explícita 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 convierte en buenos candidatos para la inmutabilidad), me inclino a simplemente mover más 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 las transiciones de estado.

    
respondido por el Telastyn 19.08.2013 - 18:33
14
  

cuando los métodos no son nombrados apropiadamente

En realidad, ese es el verdadero olor del código. Si tienes un objeto mutable, proporciona métodos para cambiar su estado. Si tiene una llamada a un método de este tipo integrado en una tarea de algunas declaraciones más, está bien refactorizar esa tarea a un método propio, lo que lo deja en la situación exacta descrita. Pero si no tienes nombres de métodos como Foo y Bar , pero hay métodos que dejan en claro que cambian el objeto, no veo un problema aquí. Piensa 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 aprobado.

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

    
respondido por el Doc Brown 19.08.2013 - 21:01
3

Sí, consulte enlace para uno de muchos ejemplos de personas que señalan que los efectos secundarios inesperados son malos.

En general, el principio fundamental es que el software está integrado 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 hay que tener en cuenta lo menos posible para usarla. Esto se llama modularidad y se aplica a todo, desde funciones únicas a protocolos en red.

    
respondido por el btilly 19.08.2013 - 18:36
3

En primer lugar, esto no depende de la "naturaleza de transferencia por referencia", sino que los objetos son tipos de referencia mutables. En lenguajes no funcionales, casi siempre va a ser el caso.

En segundo lugar, si esto es un problema o no, depende tanto del objeto como de la fuerza con que los cambios en los diferentes procedimientos están relacionados entre sí, si no logras hacer un cambio en Foo y eso provoca que Bar se bloquee, entonces es un problema. No es necesariamente un olor a código, pero es un problema con Foo o con Bar o con Algo (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 estar evitando).

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

    
respondido por el jmoreno 19.08.2013 - 20:35
2

Yo diría que hay poca diferencia entre A.Do(Something) modificando something y something.Do() modificando something . En el caso cualquiera , debe quedar claro en el nombre del método invocado que se modificará something . Si no queda claro en el nombre del método, independientemente de si something es un parámetro, this o parte del entorno, no debería ser modificado.

    
respondido por el svidgen 19.08.2013 - 22:45
1

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í, puedes hacer esto bonito moviendo el filtrado a otro método, por lo que terminarás con algo en las líneas de:

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

Donde Filter(users) se ejecutaría por encima de los filtros.

No recuerdo exactamente dónde me encontré con esto antes, pero creo que se lo llamó canalización de filtrado.

    
respondido por el CodeART 23.08.2013 - 16:07
0

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

Digamos que escribo una operación matemática compleja como una función f () . Documenté que f () es una función que asigna NXN a N , y el algoritmo detrás. Si la función se nombra de manera inadecuada, no está documentada y no tiene casos de prueba adjuntos, y tendrá que entender el código, en cuyo caso el código es inútil.

Sobre tu solución, algunas observaciones:

  • Las aplicaciones se diseñan 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 rellenarlo con detalles sobre cómo cambiarlo.
  • La clonación de objetos conlleva a requisitos de memoria hinchada, y en muchos casos lleva a la existencia de objetos equivalentes en estados incompatibles ( X se convirtió en Y después de f() , pero X es en realidad Y ,) y posiblemente inconsistencia temporal.

El problema que está tratando de resolver es válido; sin embargo, incluso con una enorme sobreingeniería realizada, el problema se evita, no se resuelve.

    
respondido por el CMR 19.08.2013 - 19:57

Lea otras preguntas en las etiquetas