Pregunta

Digamos que tienes estos dos métodos:

Número 1:

void AddPerson(Person person)
{
  // Validate person
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

Número 2:

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  DB.AddPersonToDatabase(person);
}

¿Cuál de los dos métodos es el mejor? Sé que el primero es más correcto en cuanto a OO, pero creo que el segundo es más legible, y no tiene que asegurarse de que el objeto sea válido como lo aseguran los parámetros. Simplemente no me gusta tener que validar los objetos en cualquier lugar, los paso como parámetros. ¿Hay otros enfoques?

EDITAR: Gracias por todas las respuestas. Para aclarar, la validación en el constructor y un método IsValid es, por supuesto, un buen enfoque, pero en mi código, el estado válido de la persona a menudo depende del contexto y puede variar de un método a otro. Por supuesto, esto podría ser un signo de mal diseño.

El código es solo un ejemplo para describir el problema.

¿Fue útil?

Solución

El primero no debería tener que validar person.Name y person.BirthDate: deben ser validados automáticamente por el constructor Person . En otras palabras, si se te pasa una persona, debes saber que es válida.

Por otro lado, deberías comprobar que person no es una referencia nula.

A veces vale la pena tener métodos de conveniencia como la segunda versión para evitar tener que llamar explícitamente al constructor con la misma frecuencia. Por lo general, solo debe llamar al constructor Person y luego delegar el trabajo al primer formulario.

Otros consejos

El primero tiene la ventaja de permitirle cambiar la definición de Persona sin romper el código existente, solo se necesita la recompilación. Puede pensar que la segunda es más legible, pero la primera es más fácil de mantener, su elección.

Otra opción sería:

void AddPerson(Person person)
{  // Validate person  
   if(person.IsValid)
   {
     DB.AddPersonToDatabase(person);
   }
}

Suponiendo que la persona no se valida a sí misma cuando se construye. Lo que en algunos casos es viable si el objeto puede tener un estado no válido mientras sea transitorio.

Prefiero el anterior (pasar un objeto) porque reduce el acoplamiento de la API al objeto. Si cambia el objeto Person , por ejemplo, agregue una nueva propiedad como Nickname que no era necesaria anteriormente, entonces en el primer caso no necesita cambiar la API pública, mientras que en la segunda necesita cambiar el método o agregar una nueva sobrecarga.

Estoy de acuerdo en que depende completamente del contexto, no hay reglas absolutas para esto. En mi opinión, sería absurdo tener métodos como estos:

person.SetBirthDate(Person person)
person.ResetPassword(Person person)

Pero en este caso prefiero el primero, porque, como dijo Greg Beech, el método no (tiene que) saber nada sobre el objeto de dominio.

Por cierto, considere la sobrecarga (SECO - No se repita):

void AddPerson(Person person)
{
  if(person.Name != null && IsValidDate(person.BirthDate)
    DB.AddPersonToDatabase(person);
}

void AddPerson(string name, DateTime birthDate)
{
  Person p = new Person(name, birthDate);
  this.AddPerson(p);
}

Definitivamente es mejor pasar un objeto Person, en lugar de un grupo de tipos primitivos como parámetros. Compara los siguientes dos métodos


public static void Withdrawal(Account account, decimal amount)
{
    DB.UpdateBalance(account.AccountNumber, amount);
}

public static void Withdraw(int accountNumber, decimal amount)
{
    DB.UpdateBalance(accountNumber, amount);
}

Los dos métodos parecen casi idénticos, pero el segundo no es seguro. Un int puede venir de cualquier parte, por lo que te equivocan si escribes esto:

private void CloseTransaction(Transaction tran)
{
    BankAccounts.Withdrawal(tran.Account.RoutingNumber, tran.Amount);
        // logic error: meant to pass Account.AccountNumber instead of Account.RoutingNumber
}

Este es el peor tipo de error porque no generará un error de compilación o una excepción de tiempo de ejecución. Puede detectar este error en sus pruebas automatizadas si las escribe lo suficientemente bien, pero este error es fácil de omitir y puede ocultarse durante meses sin ser descubierto.

Trabajé en una empresa que escribía software bancario, y realmente nos encontramos con un error de este tipo en la producción. Solo ocurrió durante un tipo específico de transferencia de bóveda, y solo se descubrió cuando uno de nuestros bancos notó sus saldos de GL por unos $ 100 cada vez que ejecutaron su proceso de fin de mes. El banco sospechó el robo de empleados durante meses, pero fue solo a través de una cuidadosa revisión del código que alguien detectó el problema hasta un error en nuestro software.

La mayor parte del tiempo iría por el primero.

El segundo rompería la firma por cada cambio en Persona

Depende del contexto.

Si todos sus métodos de llamada tratan con objetos Person, entonces la primera es la solución correcta.

Sin embargo, si algunos de sus métodos de llamada se relacionan con el nombre y las fechas de nacimiento y algunos tratan con los objetos Person, entonces el segundo ejemplo es la solución correcta.

Simplemente crearía sobrecargas para ambos. Especialmente teniendo en cuenta que se pueden crear automáticamente.

Supongo que los métodos no forman parte del tipo Person . Teniendo esto en cuenta, siento que ambos tienen demasiado conocimiento sobre otro tipo ( Person ) imo. La primera versión no debería tener que validar la creación de una instancia de persona válida. Si esto es responsabilidad de la persona que llama, entonces cada persona que llama debe hacer esto. Eso es frágil.

La segunda versión tiene una fuerte dependencia de otro tipo debido al hecho de que crea una instancia de este tipo.

Definitivamente preferiría la primera versión, pero movería la parte de validación de esa parte del código.

Creo que Jon casi lo clavó. Person debe ser responsabilidad de garantizar que una persona válida se crea en su constructor.

Acerca de la resonsibilidad que crea o no crea el objeto Person (ya sea el método AddPerson o su llamante), lea

http://en.wikipedia.org/wiki/GRASP_%28Object_Oriented_Design % 29 # creador

Se trata de cuestiones de responsabilidad en OOP. En su caso específico, si AddPerson envuelve la llamada a una interfaz DB, no estoy muy seguro de ello. Depende para qué se utiliza ese objeto Person fuera de ese contexto. Si es solo con el propósito de contener los datos que se agregarán a la Base de datos, crearlos en el método AddPerson posiblemente sea una buena idea, ya que desacopla al usuario de su clase de tener que saber sobre la clase Persona. .

Mejor encapsulación si usas objetos; para eso están Creo que la gente se mete en problemas cuando usan primitivos demasiado.

No debería ser posible crear una Persona inválida. El constructor debe verificar los parámetros válidos y lanzar la excepción IllegalArgumentException o fallar una afirmación si no son válidos. De esto se trata la Programación por Contrato.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top