Aplicabilidad del principio de responsabilidad única

Recientemente me topé con un problema arquitectónico aparentemente trivial. Tenía un repositorio simple en mi código que se llamaba así (el código está en C #):

var user = /* create user somehow */;
_userRepository.Add(user);
/* do some other stuff*/
_userRepository.SaveChanges();

SaveChanges fue un contenedor simple que confirma los cambios en la base de datos:

void SaveChanges()
{
    _dataContext.SaveChanges();
    _logger.Log("User DB updated: " + someImportantInfo);
}

Luego, después de un tiempo, necesitaba implementar una nueva lógica que enviara notificaciones por correo electrónico cada vez que se creaba un usuario en el sistema. Como había muchas llamadas hacia _userRepository.Add()y SaveChangesalrededor del sistema, decidí actualizar de SaveChangesesta manera:

void SaveChanges()
{
    _dataContext.SaveChanges();
    _logger.Log("User DB updated: " + someImportantInfo);
    foreach (var newUser in dataContext.GetAddedUsers())
    {
       _eventService.RaiseEvent(new UserCreatedEvent(newUser ))
    }
}

De esta forma, el código externo podría suscribirse a UserCreatedEvent y manejar la lógica de negocios necesaria que enviaría notificaciones.

Pero se me señaló que mi modificación de SaveChangesviolaba el principio de Responsabilidad Única, y eso SaveChangessolo debería salvar y no disparar ningún evento.

¿Es este un punto válido? Me parece que la generación de un evento aquí es esencialmente lo mismo que el registro: simplemente agregando algunas funciones secundarias a la función. Y SRP no le prohíbe usar eventos de inicio de sesión o disparo en sus funciones, solo dice que dicha lógica debe encapsularse en otras clases, y está bien que un repositorio llame a estas otras clases.

Sí, puede ser un requisito válido tener un repositorio que active ciertos eventos en ciertas acciones como Addo SaveChanges, y no voy a cuestionar esto (como algunas otras respuestas) solo porque su ejemplo específico de agregar usuarios y enviar correos electrónicos puede parecer un poco artificial A continuación, supongamos que este requisito está perfectamente justificado en el contexto de su sistema.

Entonces , sí , codificar la mecánica del evento, así como el registro y el guardado en un método viola el SRP . Para muchos casos, es probablemente una violación aceptable, especialmente cuando nadie quiere distribuir las responsabilidades de mantenimiento de "guardar cambios" y "elevar evento" a diferentes equipos / mantenedores. Pero supongamos que un día alguien quiere hacer exactamente esto, ¿se puede resolver de una manera simple, quizás poniendo el código de esas preocupaciones en diferentes bibliotecas de clases?

La solución a esto es dejar que su repositorio original siga siendo responsable de enviar cambios a la base de datos, nada más, y hacer un repositorio proxy que tenga exactamente la misma interfaz pública, reutilice el repositorio original y agregue la mecánica de eventos adicional a los métodos.

// In EventFiringUserRepo:
public void SaveChanges()
{
  _basicRepo.SaveChanges();
   FireEventsForNewlyAddedUsers();
}

private void FireEventsForNewlyAddedUsers()
{
  foreach (var newUser in _basicRepo.DataContext.GetAddedUsers())
  {
     _eventService.RaiseEvent(new UserCreatedEvent(newUser))
  }
}

Puede llamar a la clase proxy a, NotifyingRepositoryo ObservableRepositorysi lo desea, siguiendo las líneas de la respuesta altamente votada de @Pedter (que en realidad no dice cómo resolver la violación de SRP, solo dice que la violación está bien).

La clase de repositorio nueva y la antigua deben derivar de una interfaz común, como se muestra en la descripción clásica del patrón Proxy .

Luego, en su código original, inicialícelo _userRepositorypor un objeto de la nueva EventFiringUserRepoclase. De esa manera, mantienes el repositorio original separado de la mecánica del evento. Si es necesario, puede hacer que el evento active el repositorio y el repositorio original uno al lado del otro y deje que las personas que llaman decidan si usan el primero o el segundo.

Para abordar una inquietud mencionada en los comentarios: ¿eso no conduce a proxies sobre proxies sobre proxies, y así sucesivamente? En realidad, agregar la mecánica del evento crea una base para agregar requisitos adicionales del tipo "enviar correos electrónicos" simplemente suscribiéndose a los eventos, por lo que también se adhiere al SRP con esos requisitos, sin ningún proxy adicional. Pero lo único que debe agregarse una vez aquí es la mecánica del evento en sí.

Si este tipo de separación realmente vale la pena en el contexto de su sistema, es algo que usted y su revisor deben decidir por sí mismos. Probablemente no separaría el registro del código original, ni usando otro proxy ni agregando un registrador al evento de escucha, aunque sería posible.

Enviar una notificación de que el almacén de datos persistente ha cambiado parece algo sensato al guardar.

Por supuesto, no debe tratar Agregar como un caso especial: también debería activar eventos para Modificar y Eliminar. Es el tratamiento especial del caso "Agregar" lo que huele, obliga al lector a explicar por qué huele, y finalmente lleva a algunos lectores del código a concluir que debe violar SRP.

Un repositorio de "notificación" que se puede consultar, cambiar y disparar eventos en los cambios, es un objeto perfectamente normal. Puede esperar encontrar múltiples variaciones de los mismos en casi cualquier proyecto de tamaño decente.

¿Pero es un repositorio de "notificaciones" lo que realmente necesita? Usted mencionó C #: Muchas personas estarían de acuerdo en que usar un System.Collections.ObjectModel.ObservableCollection<>lugar en lugar de System.Collections.Generic.List<>cuando lo último es todo lo que necesita es todo tipo de cosas malas e incorrectas, pero pocas señalarían de inmediato a SRP.

Lo que estás haciendo ahora es cambiar tu UserList _userRepositorypor un ObservableUserCollection _userRepository. Si ese es el mejor curso de acción o no, depende de la aplicación. Pero aunque sin duda hace que el _userRepositoryconsiderablemente menos liviano, en mi humilde opinión, no viola SRP.

Sí, es una violación del principio de responsabilidad única y un punto válido.

Un mejor diseño sería tener un proceso separado para recuperar 'nuevos usuarios' del repositorio y enviar los correos electrónicos. Realizar un seguimiento de qué usuarios han recibido un correo electrónico, fallas, reenvíos, etc., etc.

De esta forma, puede manejar errores, bloqueos y cosas similares, así como evitar que su repositorio tome todos los requisitos que tienen la idea de que los eventos suceden "cuando algo se compromete con la base de datos".

El repositorio no sabe que un usuario que agregue es un usuario nuevo. Su responsabilidad es simplemente almacenar al usuario.

Probablemente valga la pena ampliar los comentarios a continuación.

El repositorio no sabe que un usuario que agregue es un usuario nuevo; sí, tiene un método llamado Agregar. Su semántica implica que todos los usuarios agregados son usuarios nuevos. Combine todos los argumentos pasados ​​a Agregar antes de llamar a Guardar, y obtendrá todos los nuevos usuarios

Incorrecto. Estás combinando "Agregado al repositorio" y "Nuevo".

"Agregado al repositorio" significa exactamente lo que dice. Puedo agregar y eliminar y volver a agregar usuarios a varios repositorios.

"Nuevo" es el estado de un usuario definido por las reglas comerciales.

Actualmente, la regla de negocio podría ser "Nuevo == recién agregado al repositorio", pero eso no significa que no sea una responsabilidad separada conocer y aplicar esa regla.

Debe tener cuidado para evitar este tipo de pensamiento centrado en la base de datos. Tendrá procesos de casos extremos que agregarán usuarios no nuevos al repositorio y cuando les envíe correos electrónicos, toda la empresa dirá "¡Por supuesto que no son usuarios 'nuevos'! La regla real es X".

Esta respuesta es en mi humilde opinión: el repositorio es exactamente el único lugar central en el código que sabe cuándo se agregan nuevos usuarios

Incorrecto. Por las razones anteriores, además, no es una ubicación central a menos que realmente incluya el código de envío de correo electrónico en la clase en lugar de simplemente generar un evento.

Tendrá aplicaciones que usan la clase de repositorio, pero no tienen el código para enviar el correo electrónico. Cuando agrega usuarios en esas aplicaciones, no se enviará el correo electrónico.

¿Es este un punto válido?

Sí lo es, aunque depende mucho de la estructura de su código. No tengo el contexto completo, así que intentaré hablar en general.

Me parece que la generación de un evento aquí es esencialmente lo mismo que el registro: simplemente agregando algunas funciones secundarias a la función.

Absolutamente no lo es. El registro no es parte del flujo de negocios, se puede deshabilitar, no debe causar efectos secundarios (comerciales) y no debe influir en el estado y la salud de su aplicación de ninguna manera, incluso si por alguna razón no pudo iniciar sesión nada más. Ahora compara eso con la lógica que agregaste.

Y SRP no le prohíbe el uso de eventos de inicio de sesión o disparo en sus funciones, solo dice que dicha lógica debería encapsularse en otras clases, y está bien que un repositorio llame a estas otras clases.

SRP funciona en conjunto con ISP (S e I en SOLID). Terminas con muchas clases y métodos que hacen cosas muy específicas y nada más. Están muy enfocados, son muy fáciles de actualizar o reemplazar, y en general son más fáciles de probar. Por supuesto, en la práctica también tendrá algunas clases más grandes que se ocupan de la orquestación: tendrán una serie de dependencias y se centrarán no en acciones atomizadas, sino en acciones comerciales, que pueden requerir múltiples pasos. Siempre y cuando el contexto comercial sea claro, también se les puede llamar responsabilidad única, pero como usted dijo correctamente, a medida que el código crece, es posible que desee abstraerlo en nuevas clases / interfaces.

Ahora volvamos a su ejemplo particular. Si absolutamente debe enviar una notificación cada vez que se crea un usuario y tal vez incluso realizar otras acciones más especializadas, entonces podría crear un servicio separado que encapsule este requisito, algo así como UserCreationService, que expone un método Add(user), que maneja tanto el almacenamiento (la llamada a su repositorio) y la notificación como una sola acción comercial. O hazlo en tu fragmento original, después de_userRepository.SaveChanges();

SRP es, teóricamente, sobre personas , como explica el tío Bob en su artículo El principio de responsabilidad única . _{Gracias Robert Harvey por proporcionarlo en tu comentario.}

La pregunta correcta es:

¿Qué "parte interesada" agregó el requisito de "enviar correos electrónicos"?

Si esa parte interesada también está a cargo de la persistencia de los datos (poco probable pero posible), entonces esto no viola el SRP. De lo contrario, lo hace.

Si bien técnicamente no hay nada de malo en los repositorios que notifican eventos, sugeriría mirarlo desde un punto de vista funcional donde su conveniencia suscite algunas preocupaciones.

Crear un usuario, decidir qué es un nuevo usuario y su persistencia son 3 cosas diferentes .

^{Premisa mía}

Tenga en cuenta la premisa anterior antes de decidir si el repositorio es el lugar adecuado para notificar eventos empresariales (independientemente del SRP). Tenga en cuenta que dije evento de negocios porque para mí UserCreatedtiene una connotación diferente de UserStoredoUserAdded ¹ . También consideraría que cada uno de esos eventos está dirigido a diferentes audiencias.

Por un lado, la creación de usuarios es una regla específica del negocio que puede o no implicar persistencia. Puede involucrar más operaciones comerciales, involucrando más operaciones de base de datos / red. Operaciones que la capa de persistencia desconoce. La capa de persistencia no tiene suficiente contexto para decidir si el caso de uso finalizó con éxito o no.

Por otro lado, no es necesariamente cierto que _dataContext.SaveChanges();haya persistido al usuario con éxito. Dependerá de la duración de la transacción de la base de datos. Por ejemplo, podría ser cierto para bases de datos como MongoDB, cuyas transacciones son atómicas, pero no podría serlo, para RDBMS tradicionales que implementan transacciones ACID donde podría haber más transacciones involucradas y aún por comprometerse.

¿Es este un punto válido?

Podría ser. Sin embargo, me atrevería a decir que no es solo una cuestión de SRP (técnicamente hablando), sino también una cuestión de conveniencia (funcionalmente hablando).

• ¿Es conveniente disparar eventos comerciales desde componentes que desconocen las operaciones comerciales en curso?
• ¿Representan el lugar correcto tanto como el momento adecuado para hacerlo?
• ¿Debo permitir que estos componentes orquenen mi lógica empresarial a través de notificaciones como esta?
• ¿Podría invalidar los efectos secundarios causados ​​por eventos prematuros? ²

Me parece que la aparición de un evento aquí es esencialmente lo mismo que iniciar sesión

Absolutamente no. Sin embargo, el registro no debe tener efectos secundarios, ya que sugirió que UserCreatedes probable que el evento provoque otras operaciones comerciales. Como notificaciones. ³

solo dice que dicha lógica debería encapsularse en otras clases, y está bien que un repositorio llame a estas otras clases

No necesariamente cierto. SRP no es una preocupación específica de clase solamente. ¡Funciona en diferentes niveles de abstracciones, como capas, bibliotecas y sistemas! Se trata de la cohesión, de mantener juntos lo que cambia por las mismas razones de la mano de los mismos interesados . Si la creación del usuario ( caso de uso ) cambia, es probable que el momento y los motivos del evento también cambien.

^{1: Nombrar las cosas adecuadamente también es importante.}

^{2: Digamos que enviamos UserCreateddespués _dataContext.SaveChanges();, pero la transacción completa de la base de datos falló más tarde debido a problemas de conexión o violaciones de restricciones. Tenga cuidado con la transmisión prematura de eventos, porque sus efectos secundarios pueden ser difíciles de deshacer (si eso es posible).}

3: Los procesos de notificación que no se tratan adecuadamente pueden provocar que se activen notificaciones que no se pueden deshacer / sup>

No, esto no viola el SRP.

Muchos parecen pensar que el Principio de Responsabilidad Única significa que una función solo debe hacer "una cosa", y luego quedar atrapados en la discusión sobre lo que constituye "una cosa".

Pero eso no es lo que significa el principio. Se trata de preocupaciones a nivel empresarial. Una clase no debe implementar múltiples preocupaciones o requisitos que pueden cambiar de forma independiente a nivel comercial. Digamos que una clase almacena al usuario y envía un mensaje de bienvenida codificado por correo electrónico. Múltiple independiente preocupaciones podrían causar que los requisitos de dicha clase cambien. El diseñador podría requerir que la hoja de estilo / html del correo cambie. El experto en comunicaciones podría requerir que se modifique la redacción del correo. Y el experto en UX podría decidir que el correo debería enviarse en un punto diferente del flujo de incorporación. Por lo tanto, la clase está sujeta a múltiples cambios de requisitos de fuentes independientes. Esto viola el SRP.

Pero disparar un evento no viola el SRP, ya que el evento solo depende de salvar al usuario y no de ninguna otra preocupación. Los eventos son en realidad una buena manera de mantener el SRP, ya que puede guardar un correo electrónico activado por el guardado sin que el repositorio se vea afectado, o incluso saber, el correo.

No te preocupes por el principio de responsabilidad única. No va a ayudarlo a tomar una buena decisión aquí porque puede elegir subjetivamente un concepto particular como "responsabilidad". Podría decir que la responsabilidad de la clase es administrar la persistencia de datos en la base de datos, o podría decir que su responsabilidad es realizar todo el trabajo relacionado con la creación de un usuario. Estos son solo niveles diferentes del comportamiento de la aplicación, y ambos son expresiones conceptuales válidas de una "responsabilidad única". Por lo tanto, este principio no es útil para resolver su problema.

El principio más útil para aplicar en este caso es el principio de menor sorpresa . Entonces, hagamos la pregunta: ¿es sorprendente que un repositorio con la función principal de datos persistentes en una base de datos también envíe correos electrónicos?

Sí, es muy sorprendente. Estos son dos sistemas externos completamente separados, y el nombre SaveChangesno implica también enviar notificaciones. El hecho de que delegue esto en un evento hace que el comportamiento sea aún más sorprendente, ya que alguien que lee el código ya no puede ver fácilmente qué comportamientos adicionales se invocan. La indirección perjudica la legibilidad. A veces, los beneficios valen los costos de legibilidad, pero no cuando se invoca automáticamente un sistema externo adicional que tiene efectos observables para los usuarios finales. (El registro puede excluirse aquí ya que su efecto es esencialmente el mantenimiento de registros con fines de depuración. Los usuarios finales no consumen el registro, por lo que no hay ningún daño en el registro siempre.) Aún peor, esto reduce la flexibilidad en el tiempo de enviar el correo electrónico, lo que hace imposible intercalar otras operaciones entre guardar y la notificación.

Si su código generalmente necesita enviar una notificación cuando un usuario se crea con éxito, puede crear un método que lo haga:

public void AddUserAndNotify(IUserRepository repo, IEmailNotification notifier, MyUser user)
{
    repo.Add(user);
    repo.SaveChanges();
    notifier.SendUserCreatedNotification(user);
}

Pero si esto agrega valor depende de los detalles de su aplicación.

De hecho, desalentaría la existencia del SaveChangesmétodo. Este método presumiblemente confirmará una transacción de la base de datos, pero otros repositorios podrían haber modificado la base de datos en la misma transacción . El hecho de que los compromete a todos nuevamente es sorprendente, ya que SaveChangesestá específicamente relacionado con esta instancia del repositorio de usuarios.

El patrón más directo para administrar una transacción de base de datos es un usingbloque externo :

using (DataContext context = new DataContext())
{
    _userRepository.Add(context, user);
    context.SaveChanges();
    notifier.SendUserCreatedNotification(user);
}

Esto le da al programador un control explícito sobre cuándo se guardan los cambios para todos los repositorios, obliga al código a documentar explícitamente la secuencia de eventos que deben ocurrir antes de una confirmación, asegura que se emita una reversión por error (suponiendo que se DataContext.Disposeemita una reversión) y evita que se oculte conexiones entre clases con estado.

También preferiría no enviar el correo electrónico directamente en la solicitud. Sería más robusto registrar la necesidad de una notificación en una cola. Esto permitiría un mejor manejo de fallas. En particular, si se produce un error al enviar el correo electrónico, se puede volver a intentar más tarde sin interrumpir el guardado del usuario, y se evita el caso en el que se crea el usuario pero el sitio devuelve un error.

using (DataContext context = new DataContext())
{
    _userRepository.Add(context, user);
    _emailNotificationQueue.AddUserCreateNotification(user);
    _emailNotificationQueue.Commit();
    context.SaveChanges();
}

Es mejor confirmar primero la cola de notificaciones, ya que el consumidor de la cola puede verificar que el usuario existe antes de enviar el correo electrónico, en caso de que context.SaveChanges()falle la llamada. (De lo contrario, necesitará una estrategia de compromiso de dos fases completa para evitar errores de seguridad).

El resultado final es ser práctico. Realmente piense en las consecuencias (tanto en términos de riesgo como de beneficio) de escribir código de una manera particular. Me parece que el "principio de responsabilidad única" no me ayuda mucho a hacerlo, mientras que el "principio de menor sorpresa" a menudo me ayuda a meterme en la cabeza de otro desarrollador (por así decirlo) y pensar en lo que podría suceder.

Actualmente SaveChangeshace dos cosas: guarda los cambios y registra que lo hace. Ahora desea agregarle otra cosa: enviar notificaciones por correo electrónico.

Tuviste la inteligente idea de agregarle un evento, pero esto fue criticado por violar el Principio de Responsabilidad Única (SRP), sin notar que ya se había violado.

Para obtener una solución SRP pura, primero active el evento, luego llame a todos los ganchos para ese evento, de los cuales ahora hay tres: guardar, iniciar sesión y finalmente enviar correos electrónicos.

O disparas el evento primero o tienes que agregarlo SaveChanges. Su solución es un híbrido entre los dos. No aborda la violación existente, mientras que alienta a evitar que aumente más allá de tres cosas. Refactorizar el código existente para cumplir con SRP puede requerir más trabajo del estrictamente necesario. Depende de su proyecto qué tan lejos quieren llevar SRP.

El código ya violaba el SRP: la misma clase era responsable de comunicarse con el contexto de datos y el registro.

Simplemente actualícelo a 3 responsabilidades.

Una forma de despojar las cosas a una responsabilidad sería abstraer el _userRepository; conviértalo en un emisor de comandos.

Tiene un conjunto de comandos, más un conjunto de oyentes. Obtiene comandos y los transmite a sus oyentes. Posiblemente esos oyentes están ordenados, y tal vez incluso puedan decir que el comando falló (que a su vez se transmite a los oyentes que ya habían sido notificados).

Ahora, la mayoría de los comandos pueden tener solo 1 oyente (el contexto de datos). SaveChanges, antes de sus cambios, tiene 2: el contexto de datos y luego el registrador.

Su cambio luego agrega otro oyente para guardar los cambios, que es generar nuevos eventos creados por el usuario en el servicio de eventos.

Hay algunos beneficios para esto. Ahora puede eliminar, actualizar o replicar el código de registro sin que le importe el resto del código. Puede agregar más desencadenantes en los cambios guardados para más cosas que lo necesitan.

Todo esto se decide cuando _userRepositoryse crea y se conecta (o, tal vez, esas características adicionales se agregan / eliminan sobre la marcha; poder agregar / mejorar el registro mientras se ejecuta la aplicación podría ser útil).