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(); 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); } 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 )) } } 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.