Single Responsibility Principle – Applicability in Software Architecture

Architectureeventsingle-responsibility

I recently came by a seemingly trivial architectural problem. I had a simple repository in my code that was called like this (code is in C#):

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

SaveChanges was a simple wrapper that commits changes to database:

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

Then, after some time, I needed to implement new logic that would send email notifications every time a user was created in the system. Since there were many calls to _userRepository.Add() and SaveChanges around the system, I decided to update SaveChanges like this:

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

This way, external code could subscribe to UserCreatedEvent and handle the needed business logic that would send notifications.

But it was pointed out to me that my modification of SaveChanges violated the Single Responsibility principle, and that SaveChanges should just save and not fire any events.

Is this a valid point? It seems to me that the raising an event here is essentially the same thing as logging: just adding some side functionality to the function. And SRP does not prohibit you from using logging or firing events in your functions, it just says that such logic should be encapsulated in other classes, and it is OK for a repository to call these other classes.

Best Answer

Yes, it can be a valid requirement to have a repository which fires certain events on certain actions like Add or SaveChanges - and I am not going to question this (like some other answers) just because your specific example of adding users and sending emails may look a bit contrived. In the following, let us assume this requirement is perfectly justified in the context of your system.

So yes, encoding the event mechanics as well as the logging as well as the saving into one method violates the SRP. For lots of cases , it is probably an acceptable violation, especially when noone ever wants to distribute the maintenance responsibilities of "save changes" and "raise event" to different teams/maintainers. But lets assume one day someone wants to do exactly this, can it be resolved in a simple manner, maybe by putting the code of those concerns into different class libraries?

The solution to this is to let your original repository stay responsible for committing changes to database, nothing else, and make a proxy repository which has exactly the same public interface, reuses the original repository and adds the additional event mechanics to the methods. 

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

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

You can call the proxy class a NotifyingRepository or ObservableRepository if you like, along the lines of @Peter's highly voted answer (which actually does not tell how to resolve the SRP violation, only saying the violation is ok).

The new and the old repository class should both derive from a common interface, like shown in the classic Proxy pattern description.

Then, in your original code, initialize _userRepository by an object of the new EventFiringUserRepo class. That way, you keep the original repository separated from the event mechanics. If required, you can have the event firing repository and the original repository side-by-side and let the callers decide if they use either the former or the latter.

To adress one concern mentioned in the comments: doesn't that lead to proxies on top of proxies on top of proxies, and so on? Actually, adding the event mechanics creates a foundation to add further requirements of the "send emails" type by simply subscribing to the events, so sticking to the SRP with those requirements as well, without any additional proxies. But the one thing which has to be added once here is the event mechanics itself.

If this kind of separation is really worth it in the context of your system is something you and your reviewer have to decide by yourself. I probably would not separate the logging from the original code, neither by using another proxy not by adding a logger to the listener event, though it would be possible.

Related Topic