¿Es un código malicioso que un método dependa de otro?

Question

Estoy refactorizando una clase para que el código sea comprobable (usando NUnit y RhinoMocks como framework de pruebas y aislamientos) y encontré que he encontrado que un método depende de otro (es decir, depende de algo creado por ese otro método). Algo así como lo siguiente:

public class Impersonator 
{ 
    private ImpersonationContext _context; 

    public void Impersonate() 
    { 
     ... 
     _context = GetContext(); 
     ... 
    } 

    public void UndoImpersonation() 
    { 
     if (_context != null) 
      _someDepend.Undo(); 
    } 
} 

lo que significa que a prueba de UndoImpersonation, necesito configurarlo llamando Impersonate (Impersonate ya cuenta con varias pruebas de unidad para verificar su comportamiento). Esto huele mal en mí, sino en cierto sentido, tiene sentido desde el punto de vista del código que llama a esta clase:

public void ExerciseClassToTest(Impersonator c) 
{ 
    try 
    { 
     if (NeedImpersonation()) 
     { 
      c.Impersonate(); 
     } 
     ... 
    } 
    finally 
    { 
     c.UndoImpersonation(); 
    } 
} 

no habría trabajado esto si yo no trato de escribir una prueba de unidad para UndoImpersonation y me encontré teniendo que configurar la prueba llamando al otro método público. Entonces, ¿esto es un mal olor y si es así cómo puedo evitarlo?

Answer 1

El olor del código tiene que ser uno de los más términos vagos que he encontrado en el mundo de la programación. Para un grupo de personas que se enorgullecen de los principios de la ingeniería, se clasifica en términos de basura inconmensurable, y una medida casi inútil, como los LOC por día para la eficiencia del programador.

De todos modos, esa es mi queja, gracias por escuchar :-)

Para responder a su pregunta específica, no creo que esto es un problema. Si prueba algo que tiene precondiciones, debe asegurarse de que las condiciones previas se hayan configurado primero para el caso de prueba dado.

Uno de los ensayos deberá ser lo que sucede cuando usted lo llama sin primer establecimiento las condiciones previas - debe o bien no con gracia o la creación de su propia condición previa si la persona que llama no se ha molestado en hacerlo .

Answer 2

Bueno, hay un contexto demasiado pequeño para decir, parece que _someDepend debe estar inicializado en el constructor.

La inicialización de campos en un método de instancia es un gran NO para mí. Una clase debe ser totalmente utilizable (es decir, todos los métodos funcionan) tan pronto como se construya; entonces el constructor (es) debería inicializar todas las variables de instancia. Ver p. la página en single step construction en la wiki de Ward Cunningham.

La razón por la que los campos de inicialización en un método de instancia son malos es principalmente porque impone un orden implícito sobre cómo puede llamar a los métodos. En su caso, TheMethodIWantToTest hará diferentes cosas dependiendo de si DoStuff fue llamado primero. Esto generalmente no es algo que un usuario de su clase esperaría, entonces es malo :-(.

Dicho esto, a veces este tipo de acoplamiento puede ser inevitable (por ejemplo, si un método adquiere un recurso como un identificador de archivo y se necesita otro método para liberarlo). Pero incluso eso debe ser manejado dentro de un método si es posible.

lo que se aplica a su caso es difícil de decir sin más contexto.

Answer 3

siempre que nO se consideran objetos mutables un código huele solo, tener que poner un objeto en el estado necesario para una prueba es simplemente parte de la configuración para esa prueba.

Answer 4

Esto es a menudo inevitable, por ejemplo, cuando se trabaja con las conexiones remotas - usted tiene que llamar Open() antes de poder llamar Close(), y no desea Open() suceda de forma automática en el constructor.

Sin embargo, debe tener mucho cuidado al hacer esto, ya que el patrón es algo fácil de entender, por ejemplo, creo que la mayoría de los usuarios aceptan este tipo de comportamiento para cualquier cosa transaccional, pero podrían sorprenderse al encontrar DoStuff() y TheMethodIWantToTest() (lo que sea realmente se llama).

Por lo general, es una buena práctica tener una propiedad que represente el estado actual; consulte nuevamente las conexiones remotas o de base de datos para ver un ejemplo de un diseño coherente.

El gran no-no es que esto ocurra para las propiedades. Las propiedades deberían llamar al nunca en qué orden se llaman. Si tiene un valor simple que depende del orden de los métodos, entonces debería ser un método sin parámetros en lugar de un property-get.

Answer 5

Responder el título: tener métodos para llamarse entre sí (encadenamiento) es inevitable en la programación orientada a objetos, por lo que en mi opinión no hay nada de malo en probar un método que llama a otro. Una prueba de unidad puede ser una clase después de todo, es una "unidad" que está probando.

El nivel de encadenamiento depende del diseño de su objeto - usted puede tenedor o cascada.

• bifurca: classToTest1.SomeDependency.DoSomething()
• cascada: classToTest1.DoSomething() (que internamente llamaría SomeDependency.DoSomething)

Pero, como otros han mencionado, sin duda mantener su estado de inicialización en el constructor que por lo que puedo decir, probablemente resolverá tu problema.

Answer 6

Sí, creo que hay un olor a código en este caso. No debido a las dependencias entre métodos, sino debido a la identidad vaga del objeto. En lugar de tener un Impersonator que puede estar en diferentes estados persona, ¿por qué no tener un Persona inmutable?

Si necesita una Persona diferente, simplemente cree una nueva en lugar de cambiar el estado de un objeto existente. Si necesita hacer una limpieza después, haga Persona desechable. Puede mantener a la clase Impersonator como una fábrica:

using (var persona = impersonator.createPersona(...)) 
{ 
    // do something with the persona 
}