SOLID: A refactoring example

SOLID principles have been around for a long time and there's multiple articles out there explaining it, so why do yet another article? This blog isn't really meant to teach you SOLID principles but to provide an example based on a simplified real world example I ran into while working on a Spring Boot application.

Legacy code

@Service
public class BadAccountService {

    public static final String MESSAGE = "You've been notified!";
    private EmailNotificationService emailNotificationService;
    private SmsNotificationService smsNotificationService;

    public BadAccountService(EmailNotificationService emailNotificationService,
                             SmsNotificationService smsNotificationService) {
        this.emailNotificationService = emailNotificationService;
        this.smsNotificationService = smsNotificationService;
    }

    public void notifyAccount(Account account) {

        switch (account.getNotificationPreference()) {
            case "sms":
              smsNotificationService.notify(account.getName(), MESSAGE);
              break;
            case "email":
            default:
              emailNotificationService.notify(account.getName(), MESSAGE);
        }
    }
}

Let's break this down a bit, we have an AccountService that has a notify method that intends to send a message to the account holder based on his preference. Email, or SMS. The good part of this code is that SMS notification is implemented in a different class than Email notification (Single Responsibility principle). The best part of this code is that it is easily testable, due to its usage of dependency injection, here's a simple test case:

@Test
void badAccountNotificationTest() {
   Account emailPreference = new Account("Ben", "email");
   Account smsPreference = new Account("Cecilia", "sms");
   Account unknownPreference = new Account("John", "unknown");

   badAccountService.notifyAccount(emailPreference);
   badAccountService.notifyAccount(smsPreference);
   badAccountService.notifyAccount(unknownPreference);

  verify(emailNotificationService, times(1))
         .notify("Ben", "You've been notified!");

  verify(smsNotificationService, times(1))
         .notify("Cecilia", "You've been notified!");

  verify(emailNotificationService, times(1))
         .notify("John", "You've been notified!");
    }

The bad part is that the notifyAccount method uses the two notification services directly, and uses a switch statement to direct the flow with the notification preference which violates the Open/Close principle. If we wanted to add a third notification preference we'd have to change the switch statement.

Refactoring

First thing is to extract an interface from the notification services and make the notification services implement it:

public interface NotificationService {
    void notify(String person, String message);
}

Then we'll need a Factory to remove the need for a switch:

@Component
public class NotificationFactory {

    private Map<String, NotificationService> notificationServices;
    private EmailNotificationService defaultNotificationService;

    public NotificationFactory(Map<String, NotificationService> notificationServices, EmailNotificationService defaultNotificationService){
        this.notificationServices = notificationServices;
        this.defaultNotificationService = defaultNotificationService;
    }

    public Optional<NotificationService> getNotificationService(String type) {
         return Optional.ofNullable(notificationServices.get(type));
    }

    public NotificationService getDefaultNotificationService() {
        return defaultNotificationService;
    }
}

This Factory provides two ways of getting a notification service: The default notification, which requires EmailNotificationService to be injected directly and a way to look up a notification by it's name. This lookup deserves a deeper dive.

With Spring you can inject a list or a map of Spring components that implement an interface. Our map will have the the following elements:

• email -> EmailNotificationService
• sms -> SmsNotificationService

The String portion of the map is grabbed from the spring service's name:

@Service("email")
public class EmailNotificationService implements NotificationService {

    private static final Logger log = LoggerFactory.getLogger(EmailNotificationService.class);

    @Override
    public void notify(String person, String message) {
        log.info("{} has been notified with message: {}", person, message);
    }
}

@Service("sms")
public class SmsNotificationService implements NotificationService {

    private static final Logger log = LoggerFactory.getLogger(SmsNotificationService.class);

    @Override
    public void notify(String person, String message) {
        log.info("{} has been notified with message: {}", person, message);
    }
}

Finally we can refactor our AccountService class:

@Service
public class AccountService {

    private NotificationFactory notificationFactory;

    public AccountService(NotificationFactory notificationFactory) {
        this.notificationFactory = notificationFactory;
    }

    public void notifyAccount(Account account) {
        notificationFactory
         .getNotificationService(account.getNotificationPreference())
         .orElseGet(notificationFactory::getDefaultNotificationService)
         .notify(account.getName(), "You've been notified!");
    }
}

So what did we accomplish?

We've introduced a Factory which solves our main issue, the violation of the Open/Close principle. We can now add or remove notification services without impacting AccountService. The only change that we may need to make in the future would be to change the default notification service, however this implementation is much better than the default switch case.

We've decoupled the different notification services from the AccountService and it now only depends on the NotificationFactory, and expressed the default notification method in a more explicit way than a switch fall through.

The test method is actually unchanged, so we were able to do all this without changing our test.

You can find the source code here.