Aplicabilidade do princípio de responsabilidade única

Recentemente, deparei-me com um problema arquitetônico aparentemente trivial. Eu tinha um repositório simples no meu código que foi chamado assim (o código está em C #):

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

SaveChanges era um invólucro simples que confirma as alterações no banco de dados:

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

Depois de algum tempo, eu precisei implementar uma nova lógica que enviaria notificações por email toda vez que um usuário fosse criado no sistema. Como havia muitas chamadas para _userRepository.Add()e SaveChangesao redor do sistema, decidi atualizar SaveChangesassim:

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

Dessa forma, o código externo pode se inscrever no UserCreatedEvent e manipular a lógica comercial necessária para enviar notificações.

Mas foi apontado para mim que minha modificação SaveChangesviolou o princípio de Responsabilidade Única, e isso SaveChangesdeve salvar e não disparar nenhum evento.

Este é um ponto válido? Parece-me que a criação de um evento aqui é essencialmente a mesma coisa que o log: apenas adicionando alguma funcionalidade lateral à função. E o SRP não proíbe o uso de eventos de registro ou disparo em suas funções, apenas diz que essa lógica deve ser encapsulada em outras classes e não há problema em um repositório chamar essas outras classes.

Sim, pode ser um requisito válido ter um repositório que dispara determinados eventos em determinadas ações como Addou SaveChanges- e não vou questionar isso (como algumas outras respostas) apenas porque seu exemplo específico de adição de usuários e envio de e-mails pode parecer um um pouco artificial. A seguir, vamos supor que esse requisito seja perfeitamente justificado no contexto do seu sistema.

Então , sim , a codificação da mecânica de eventos, o registro e o salvamento em um método violam o SRP . Em muitos casos, é provavelmente uma violação aceitável, especialmente quando ninguém deseja distribuir as responsabilidades de manutenção de "salvar alterações" e "aumentar evento" para diferentes equipes / mantenedores. Mas vamos supor que um dia alguém queira fazer exatamente isso, isso pode ser resolvido de uma maneira simples, talvez colocando o código dessas preocupações em diferentes bibliotecas de classes?

A solução para isso é deixar o repositório original permanecer responsável por confirmar as alterações no banco de dados, nada mais, e criar um repositório proxy que tenha exatamente a mesma interface pública, reutilizar o repositório original e adicionar a mecânica de evento adicional aos 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))
  }
}

Você pode chamar a classe proxy como NotifyingRepositoryou, ObservableRepositoryse desejar, de acordo com a resposta altamente votada de @ Peter (que na verdade não diz como resolver a violação do SRP, apenas dizendo que a violação está correta).

A classe de repositório nova e antiga devem derivar de uma interface comum, como mostrado na descrição clássica do padrão Proxy .

Em seguida, no seu código original, inicialize _userRepositorypor um objeto da nova EventFiringUserRepoclasse. Dessa forma, você mantém o repositório original separado da mecânica do evento. Se necessário, você pode ter o repositório de acionamento de eventos e o repositório original lado a lado e deixar os chamadores decidirem se usam o primeiro ou o último.

Para abordar uma preocupação mencionada nos comentários: isso não leva a proxies em cima de proxies em cima de proxies, e assim por diante? Na verdade, adicionar a mecânica de eventos cria uma base para adicionar requisitos adicionais do tipo "enviar e-mails", basta se inscrever nos eventos, aderindo ao SRP com esses requisitos também, sem proxies adicionais. Mas a única coisa que deve ser adicionada uma vez aqui é a própria mecânica do evento.

Se esse tipo de separação realmente vale a pena no contexto do seu sistema, é algo que você e seu revisor precisam decidir por si mesmos. Provavelmente não separaria o log do código original, nem usando outro proxy, não adicionando um logger ao evento do ouvinte, embora isso fosse possível.

Enviar uma notificação de que o armazenamento de dados persistente foi alterado parece uma coisa sensata a ser feita ao salvar.

É claro que você não deve tratar o Add como um caso especial - você também teria que acionar eventos para Modificar e Excluir. É o tratamento especial do caso "Adicionar" que cheira, força o leitor a explicar por que cheira e, finalmente, leva alguns leitores do código a concluir que ele deve violar o SRP.

Um repositório "de notificação" que pode ser consultado, alterado e dispara eventos em alterações é um objeto perfeitamente normal. Você pode esperar encontrar várias variações em praticamente qualquer projeto de tamanho decente.

Mas um repositório de "notificação" é realmente o que você precisa? Você mencionou o C #: muitas pessoas concordam que usar um em System.Collections.ObjectModel.ObservableCollection<>vez de System.Collections.Generic.List<>quando o último é tudo o que você precisa é de todos os tipos de coisas ruins e erradas, mas poucas apontam imediatamente para o SRP.

O que você está fazendo agora é trocar o seu UserList _userRepositorypor um ObservableUserCollection _userRepository. Se esse é o melhor curso de ação ou não, depende da aplicação. Mas, embora inquestionavelmente torne o _userRepositorypeso consideravelmente menor, na minha humilde opinião, não viola o SRP.

Sim, é uma violação do princípio de responsabilidade única e um ponto válido.

Um design melhor seria ter um processo separado para recuperar 'novos usuários' do repositório e enviar os emails. Acompanhar quais usuários receberam um email, falhas, reenvios etc., etc.

Dessa forma, você pode lidar com erros, falhas e similares, além de evitar que seu repositório atenda a todos os requisitos que tenham a ideia de que os eventos acontecem "quando algo está comprometido com o banco de dados".

O repositório não sabe que um usuário que você adiciona é um novo usuário. Sua responsabilidade é simplesmente armazenar o usuário.

Provavelmente vale a pena expandir nos comentários abaixo.

O repositório não sabe que um usuário adicionado é um novo usuário - sim, ele possui um método chamado Adicionar. Sua semântica implica que todos os usuários adicionados sejam novos. Combine todos os argumentos passados ​​para Adicionar antes de chamar Salvar - e você obtém todos os novos usuários

Incorreta. Você está confluindo "Adicionado ao repositório" e "Novo".

"Adicionado ao Repositório" significa exatamente o que diz. Posso adicionar, remover e adicionar novamente usuários a vários repositórios.

"Novo" é um estado de um usuário definido pelas regras de negócios.

Atualmente, a regra de negócios pode ser "Novo == acabado de ser adicionado ao repositório", mas isso não significa que não é uma responsabilidade separada conhecer e aplicar essa regra.

Você precisa ter cuidado para evitar esse tipo de pensamento centrado no banco de dados. Você terá processos de casos extremos que adicionam usuários não novos ao repositório e, quando enviar e-mails para eles, toda a empresa dirá "Claro que esses não são usuários 'novos'! A regra real é X"

Esta resposta está IMHO bastante errado: o repositório é exatamente o único lugar central no código que sabe quando novos usuários são adicionados

Incorreta. Pelas razões acima, além disso, não é um local central, a menos que você inclua o código de envio de email na classe em vez de apenas criar um evento.

Você terá aplicativos que usam a classe de repositório, mas não tem o código para enviar o email. Quando você adiciona usuários a esses aplicativos, o email não será enviado.

Este é um ponto válido?

Sim, apesar de depender muito da estrutura do seu código. Como não tenho o contexto completo, tentarei falar em geral.

Parece-me que a criação de um evento aqui é essencialmente a mesma coisa que o log: apenas adicionando alguma funcionalidade lateral à função.

Absolutamente não é. O log não faz parte do fluxo de negócios, pode ser desativado, não deve causar efeitos colaterais (comerciais) e não deve influenciar o estado e a integridade do seu aplicativo de maneira alguma, mesmo que por algum motivo você não tenha conseguido efetuar o log mais nada. Agora compare isso com a lógica que você adicionou.

E o SRP não proíbe o uso de eventos de registro ou disparo em suas funções, apenas diz que essa lógica deve ser encapsulada em outras classes e não há problema em um repositório chamar essas outras classes.

O SRP trabalha em conjunto com o ISP (S e I no SOLID). Você acaba com muitas classes e métodos que fazem coisas muito específicas e nada mais. Eles são muito focados, muito fáceis de atualizar ou substituir e, em geral, fáceis de testar. É claro que, na prática, você também terá algumas classes maiores que lidam com a orquestração: elas terão várias dependências e se concentrarão não em ações atomizadas, mas em ações de negócios, que podem exigir várias etapas. Desde que o contexto de negócios seja claro, eles também podem ser chamados de responsabilidade única, mas como você disse corretamente, à medida que o código cresce, convém abstrair parte dele em novas classes / interfaces.

Agora, de volta ao seu exemplo em particular. Se você absolutamente precisar enviar uma notificação sempre que um usuário for criado e talvez até executar outras ações mais especializadas, poderá criar um serviço separado que encapsule esse requisito, algo como UserCreationService, que expõe um método Add(user), que lida com o armazenamento (a chamada ao seu repositório) e a notificação como uma única ação comercial. Ou faça isso no seu snippet original, depois de_userRepository.SaveChanges();

O SRP é, teoricamente, sobre pessoas , como o tio Bob explica em seu artigo O princípio de responsabilidade única . _{Obrigado Robert Harvey por fornecê-lo em seu comentário.}

A pergunta correta é:

Qual "parte interessada" adicionou o requisito "enviar emails"?

Se essa parte interessada também é responsável pela persistência dos dados (improvável, mas possível), isso não viola o SRP. Caso contrário, ele faz.

Embora tecnicamente não haja nada de errado com os repositórios notificando eventos, sugiro analisá-lo do ponto de vista funcional, onde sua conveniência suscita algumas preocupações.

Criando um usuário, decidindo o que é um novo usuário e sua persistência são três coisas diferentes .

^{Premissa minha}

Considere a premissa anterior antes de decidir se o repositório é o local adequado para notificar eventos de negócios (independentemente do SRP). Observe que eu disse evento de negócios porque para mim UserCreatedtem uma conotação diferente de UserStoredou UserAdded ¹ . Eu também consideraria cada um desses eventos dirigidos a diferentes públicos.

Por um lado, criar usuários é uma regra específica de negócios que pode ou não envolver persistência. Pode envolver mais operações de negócios, envolvendo mais operações de banco de dados / rede. Operações das quais a camada de persistência desconhece. A camada de persistência não possui contexto suficiente para decidir se o caso de uso foi encerrado com êxito ou não.

Por outro lado, não é necessariamente verdade que _dataContext.SaveChanges();persistiu o usuário com sucesso. Depende do período de transação do banco de dados. Por exemplo, pode ser verdade para bancos de dados como o MongoDB, cujas transações são atômicas, mas não para os RDBMS tradicionais implementando transações ACID, nas quais poderia haver mais transações envolvidas e ainda a serem confirmadas.

Este é um ponto válido?

Poderia ser. No entanto, eu ousaria dizer que não é apenas uma questão de SRP (tecnicamente falando), é também uma questão de conveniência (funcionalmente falando).

• É conveniente disparar eventos de negócios de componentes que desconhecem as operações de negócios em andamento?
• Eles representam o lugar certo, tanto quanto o momento certo para fazê-lo?
• Devo permitir que esses componentes orquestrem minha lógica de negócios por meio de notificações como esta?
• Posso invalidar os efeitos colaterais causados ​​por eventos prematuros? ²

Parece-me que criar um evento aqui é essencialmente a mesma coisa que registrar

Absolutamente não. No entanto, o registro não tem efeitos colaterais, pois você sugeriu que o evento UserCreatedprovavelmente causará outras operações de negócios. Como notificações. ³

apenas diz que essa lógica deve ser encapsulada em outras classes e não há problema em um repositório chamar essas outras classes

Não é necessariamente verdade. O SRP não é apenas uma preocupação específica de classe. Opera em diferentes níveis de abstração, como camadas, bibliotecas e sistemas! Trata-se de coesão, de manter unido o que muda pelas mesmas razões, pelas mãos das mesmas partes interessadas . Se a criação do usuário ( caso de uso ) mudar, é provável que o momento e os motivos do evento também mudem.

^{1: Nomear as coisas adequadamente também é importante.}

^{2: Digamos que enviamos UserCreateddepois _dataContext.SaveChanges();, mas toda a transação do banco de dados falhou posteriormente devido a problemas de conexão ou violações de restrições. Tenha cuidado com a transmissão prematura de eventos, porque seus efeitos colaterais podem ser difíceis de desfazer (se isso for possível).}

3: Os processos de notificação não tratados adequadamente podem fazer com que você dispare notificações que não podem ser desfeitas / sup>

Não, isso não viola o SRP.

Muitos parecem pensar que o Princípio da Responsabilidade Única significa que uma função deve fazer apenas "uma coisa" e depois se envolver na discussão sobre o que constitui "uma coisa".

Mas não é isso que o princípio significa. Trata-se de preocupações em nível de negócios. Uma classe não deve implementar várias preocupações ou requisitos que possam mudar independentemente no nível de negócios. Digamos que uma classe armazene o usuário e envie uma mensagem de boas-vindas codificada por email. Várias preocupações independentes podem fazer com que os requisitos dessa classe sejam alterados. O designer pode exigir que o html / stylesheet do correio seja alterado. O especialista em comunicação pode exigir que o texto do e-mail seja alterado. E o especialista em UX pode decidir que o email deve realmente ser enviado em um ponto diferente no fluxo de integração. Portanto, a classe está sujeita a várias alterações de requisitos de fontes independentes. Isso viola o SRP.

Mas disparar um evento não viola o SRP, pois o evento depende apenas de salvar o usuário e não de qualquer outra preocupação. Os eventos são realmente uma maneira muito legal de manter o SRP, pois você pode ter um e-mail acionado pelo salvamento sem que o repositório seja afetado - ou até mesmo saiba sobre - o e-mail.

Não se preocupe com o princípio de responsabilidade única. Isso não ajudará você a tomar uma boa decisão aqui, porque você pode escolher subjetivamente um conceito específico como "responsabilidade". Você poderia dizer que a responsabilidade da classe é gerenciar a persistência de dados no banco de dados ou que a responsabilidade é executar todo o trabalho relacionado à criação de um usuário. Esses são apenas níveis diferentes do comportamento do aplicativo e são expressões conceituais válidas de uma "responsabilidade única". Portanto, este princípio é inútil para resolver seu problema.

O princípio mais útil a ser aplicado nesse caso é o princípio da menor surpresa . Então, vamos fazer a pergunta: é surpreendente que um repositório com a função principal de persistir dados em um banco de dados também envie e-mails?

Sim, é muito surpreendente. Esses são dois sistemas externos completamente separados, e o nome SaveChangesnão implica também no envio de notificações. O fato de você delegar isso a um evento torna o comportamento ainda mais surpreendente, pois alguém que lê o código não consegue mais ver facilmente quais comportamentos adicionais são chamados. O indireto prejudica a legibilidade. Às vezes, os benefícios valem os custos de legibilidade, mas não quando você invoca automaticamente um sistema externo adicional que tem efeitos observáveis ​​para os usuários finais. (O log pode ser excluído aqui, pois seu efeito é essencialmente a manutenção de registros para fins de depuração. Os usuários finais não consomem o log, portanto, não há nenhum dano em sempre o log.) Pior ainda, isso reduz a flexibilidade no tempo de enviar o email, tornando impossível intercalar outras operações entre o salvamento e a notificação.

Se seu código normalmente precisar enviar uma notificação quando um usuário for criado com sucesso, você poderá criar um método que faça isso:

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

Mas se isso agrega valor depende das especificidades do seu aplicativo.

Na verdade, eu desencorajaria a existência do SaveChangesmétodo. Esse método presumivelmente confirmará uma transação de banco de dados, mas outros repositórios podem ter modificado o banco de dados na mesma transação . O fato de confirmar todos eles é novamente surpreendente, pois SaveChangesestá especificamente vinculado a essa instância do repositório do usuário.

O padrão mais direto para gerenciar uma transação de banco de dados é um usingbloco externo :

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

Isso dá ao programador controle explícito sobre quando as alterações em todos os repositórios são salvas, força o código a documentar explicitamente a sequência de eventos que devem ocorrer antes de uma confirmação, garante que uma reversão seja emitida por erro (supondo que isso ocorra DataContext.Dispose) e evita ocultos conexões entre classes com estado.

Também prefiro não enviar o email diretamente na solicitação. Seria mais robusto registrar a necessidade de uma notificação em uma fila. Isso permitiria um melhor tratamento de falhas. Em particular, se ocorrer um erro ao enviar o email, ele poderá ser tentado novamente mais tarde, sem interromper o salvamento do usuário, e evitará o caso em que o usuário foi criado, mas um erro é retornado pelo site.

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

É melhor confirmar a fila de notificação primeiro, pois o consumidor da fila pode verificar se o usuário existe antes de enviar o email, caso a context.SaveChanges()chamada falhe. (Caso contrário, você precisará de uma estratégia de consolidação em duas fases completa para evitar erros de erro.)

A linha inferior é para ser prático. Na verdade, pense nas consequências (tanto em termos de risco quanto de benefício) de escrever código de uma maneira específica. Acho que o "princípio da responsabilidade única" não costuma me ajudar a fazer isso, enquanto o "princípio da menor surpresa" geralmente me ajuda a entrar na cabeça de outro desenvolvedor (por assim dizer) e pensar no que pode acontecer.

Atualmente, SaveChangesfaz duas coisas: salva as alterações e os logs que faz. Agora você deseja adicionar outra coisa: envie notificações por email.

Você teve a ideia inteligente de adicionar um evento a ele, mas isso foi criticado por violar o Princípio de Responsabilidade Única (SRP), sem perceber que ele já havia sido violado.

Para obter uma solução SRP pura, primeiro ative o evento e chame todos os ganchos para esse evento, dos quais existem agora três: salvar, registrar e finalmente enviar emails.

Você aciona o evento primeiro ou precisa adicionar a SaveChanges. Sua solução é um híbrido entre os dois. Não aborda a violação existente, mas incentiva a prevenção de que ela ultrapasse três aspectos. Refatorar o código existente para estar em conformidade com o SRP pode exigir mais trabalho do que o estritamente necessário. Cabe ao seu projeto até que ponto eles querem levar o SRP.

O código já violava o SRP - a mesma classe era responsável pela comunicação com o contexto e o log de dados.

Você acabou de atualizá-lo para ter 3 responsabilidades.

Uma maneira de retirar as coisas de uma responsabilidade seria abstrair a _userRepository; torná-lo um transmissor de comando.

Possui um conjunto de comandos, além de um conjunto de ouvintes. Ele recebe comandos e os transmite aos ouvintes. Possivelmente esses ouvintes estão ordenados e talvez eles possam até dizer que o comando falhou (que, por sua vez, é transmitido para ouvintes que já foram notificados).

Agora, a maioria dos comandos pode ter apenas 1 ouvinte (o contexto dos dados). SaveChanges, antes de suas alterações, possui 2 - o contexto dos dados e, em seguida, o criador de logs.

Sua alteração então adiciona outro ouvinte para salvar as alterações, que é gerar novos eventos criados pelo usuário no serviço de eventos.

Existem alguns benefícios nisso. Agora você pode remover, atualizar ou replicar o código de log sem que o restante do código seja importado. Você pode adicionar mais gatilhos nas alterações de salvamento para obter mais itens necessários.

Tudo isso é decidido quando o dispositivo _userRepositoryé criado e conectado (ou, talvez, esses recursos extras sejam adicionados / removidos em tempo real; poder adicionar / aprimorar o registro enquanto o aplicativo é executado).