Question

I have a User Model as below, this Model class is in the shared library.

class User {
    private long userId;
    private String email;
    private String userType;
    private long departmentId;

    // ... other fields and getter setters
}

In my code, I am persisting User Objects. In order to persist User object, I need to first pull the departmentId by calling an internal microservice and then save it in DB.

class UsersUpdateService {

    // Depeendency Injection
    private DepartmentServiceHttpClient departmentServiceClient;
    private UserRepository userRepository;

    public void updateUser(User user) {
        long deptId = departmentServiceClient.getDepartment(user.getUserId());
        user.setDepartmentId(deptId);
        userRepository.update(user);
    }
}

If you can see User has UserType, there are around 10 UserTypes available in the system. Now there is a new requirement if a UserType is SuperUser I need to send an email to the User after saving the object in DB.

To achieve this, I have created EmailService and I am calling emailService.sendEmail(user); after userRepository.update(user); line

public void sendEmail(User user) {
    if (user.getUserType().equals(UserType.SUPERUSER.name)) {
        //send email
    }
}

However, It looks like a hack and not a good design. Also, I think I think I am violating SRP by calling sendEmail(...) in updateEmail(...). And I think the solution will not scale well in the future, if there are requirements for different UserTypes. Also, I don't think AOP would be a good fit here.

Was it helpful?

Solution

One way to design this in a scalable way is to implement a publisher/subscriber mechanics ( for example, utilizing java.util.Observer, or by utilizing a stored lambda expression) in the UserRepository. This will allow to send a notification to an observer object (or just call the injected lambda function) whenever the UserRepository.update method is called.

The observer can check whether certain conditions are fulfilled (for example, which kind of user was updated), and then call the email sending service.

This avoids any direct coupling between the email service and the user-related classes.

Some scetched code (I am more a C# programmer, hope I got the Java syntax right):

 var emailService = new EmailService();
 // ...
 var userRepo = new UserRepository((user)-> emailService.send(user), /* other parameters */);
 // ...
 var userUpdateService=new UserUpdateService(userRepo);

 // and inside UserRepository
 class UserRepository
 {
      Consumer<User> updateCallback;    
      public UserRepository(Consumer<User> uc, /* ...*/)
      {
         updateCallback=uc;
      }
      public void updateUser(User user)
      {
          // code for updating ....
          if(updateCallback!=null)
                updateCallback(user);
      }
 }

OTHER TIPS

The main point I would raise is that the above is not OO at all. Having data structures and procedures (services) is arguably exactly the opposite of OO.

Second, please ignore SRP and SOLID. Because those (with the possible exception of L) are not well enough defined, they can't help you.

Third, coming to the problem at hand, you are using a type marker (userType) instead of polymorphism. This is a classic anti-pattern, which in this case derives probably from the User "object" being mapped directly to database.

That all being said, the proper solution would probably be polymorphism. That is, getting rid of userType and having a SuperUser class explicitly and implementing its notification logic in there.

Licensed under: CC-BY-SA with attribution
scroll top