개발자는 '빨리 나아가지 않는 방법'을 수련해야 한다.

Photo by Annie Spratt on Unsplash

 

아래는 개발하며 무의식적으로 성급함이 드러난 코드입니다.

// NotificationExecutorTest.class;

@Test
void execute_single() {

    NotificatorFactory factory = mock(NotificatorFactory.class);
    Notificator notificator = mock(Notificator.class);
    
    envelopeNotification = EnvelopeNotification.of(ANY_ID, NotificationType.SMS, NotificationFactory.create(ANY_SUBJECT, ANY_CONTENTS, ANY_DESTINATION, ANY_SOURCE));
    given(factory.findBy(NotificationType.SMS)).willReturn(notificator);
    given(notificator.execute(envelopeNotification)).willReturn(Result.ok(ANY_ID));

    Result result = sut.execute(factory, this.envelopeNotification); // Look!!!

    assertThat(result).isEqualTo(Result.ok(ANY_ID));
    verify(factory).findBy(NotificationType.SMS);
    verify(notificator).execute(envelopeNotification);
}

@Service
public class NotificationExecutor {

    public Result execute(NotificatorFactory factory, EnvelopeNotification envelopeNotification) {
        return getResult(factory, envelopeNotification);
    }

    public List<Result> execute(NotificatorFactory factory, List<EnvelopeNotification> notification) {
        return notification.stream().map(envelopeNotification -> getResult(factory, envelopeNotification))
                .collect(Collectors.toUnmodifiableList());
    }

    private Result getResult(NotificatorFactory factory, EnvelopeNotification envelopeNotification) {
        Notificator notificator = factory.findBy(envelopeNotification.getType());
        return notificator.execute(envelopeNotification);
    }
}

 

위 코드의 Sut 는 외부 모듈(ex, 뿌리고, Toast SMS, senders...)에 문자메세지를 전송하는 Executor 입니다.

 

Executor 는 '문자메세지' 타입 객체에 의존적인 형태를 가지게 됩니다. 

 

SOLID 원칙에 따르면, 확장에는 열려있고 수정에는 막혀있어야 한다고 합니다. 다시, '문자메세지' 타입 객체에 의존적인 Executor 는 만약 '문자메세지' 타입이 변경된다면 이 코드는 수정되어야 합니다.

 

왜 이런 코드를 작성하게 된걸까?

 

처음 작성했던 다이어그램은 아래와 같습니다.

다이어그램에서 Executor 는 ClientService 가 사용하는 형태를 가지게 됩니다. 

 조금더 자세히 Executor 는 NotificatorFactory 에게 타입을 알려주어 Notificator 를 가져와 execute(Notification) 를 실행합니다.

 

이게 왜 문제가 될까요?

 

Executor 는 무엇일까요? 이 객체의 역할은 Notificator 에게 적절한 객체를 넘겨주는 것이 역할이라 생각했습니다. 그런데, 작성된 테스트 코드에서 이상한 점이 있었습니다. 코드로 위 다이어그램을 POC 하는 과정에서 어색함이 발견되었습니다.

 

다시 테스트 코드를 살펴볼게요

// ExecutorTest.class;

@Test
void execute_single() {

    ...
    envelopeNotification = EnvelopeNotification.of(ANY_ID, NotificationType.SMS, NotificationFactory.create(ANY_SUBJECT, ANY_CONTENTS, ANY_DESTINATION, ANY_SOURCE));
    ...
    Result result = sut.execute(factory, this.envelopeNotification);
    ...
}

Executor 는 EnvelopeNotification.class 에 의존하게 됩니다.

문제입니다.

 

왜 구현체에 의존하는 Executor 를 만들게 되었는가? 구현체에 의존하는 것이 아닌, 필요로 하는 것에만 의존해야만 합니다.

---

이 코드는 오늘 리팩토링 되었습니다. 

 

첫번째 문제점이였던 구현체에 의존하던 문제는 인터페이스에 의존되도록 변경했습니다. 

public interface NotificationTarget {
    NotificationType getType();
}

public class EnvelopeNotification implements NotificationTarget { ... }

이렇게 인터페이스(NotificationTarget) 가 이제 Executor 가 알고 있는 유일한 객체가 됩니다.

void execute() {
        final List<NotificationTarget> notificationTargets = targetProvider.getTargets();

        for (NotificationTarget target : notificationTargets) {
            Notificator notificator = factory.findBy(target.getType());

			...
	}
 }

 

두번째 문제점은 NotificationExecutor 를 사용하는 Client 의 의도가 불분명하다 입니다.

// SchedulerService.class

@Scheduled(cron = "0 * * * * *")
public void runWithScheduled() {
    List<EnvelopeNotification> envelopeNotifications = repository.findAllByStatus(NotificationStatus.REQUESTED);
    //TODO Offset

    List<Result> results = executor.execute(envelopeNotifications);

    List<Result> sortedResults = results.stream().sorted(Comparator.comparing(o -> o.getEnvelopeNotificationId().getId())).collect(Collectors.toUnmodifiableList());
    List<EnvelopeNotification> sortedEnvelopeNotifications = envelopeNotifications.stream().sorted(Comparator.comparing(o -> o.getId().getId())).collect(Collectors.toUnmodifiableList());

    IntStream
            .range(0, Math.min(sortedResults.size(), sortedEnvelopeNotifications.size()))
            .forEach(i -> {
                        Result result = sortedResults.get(i);
                        EnvelopeNotification envelopeNotification = sortedEnvelopeNotifications.get(i);


                        if (!OK_CODE.equals(result.getCode())) {
                            repository.save(envelopeNotification.withStatus(NotificationStatus.FAILED));
                            return;
                        }

                        if (!result.getEnvelopeNotificationId().equals(envelopeNotification.getId())) {
                            return;
                        }

                        repository.save(envelopeNotification.withStatus(NotificationStatus.EXECUTING));
                    }
            );
}

스케쥴러에 의해서 EnvelopeNotification 를 가져오고 가져온 EnvelopeNotification를 executor.execute(envelopeNotifications)를 통해 실행시킵니다.

 

무엇이 문제일까요?

 

Client 에 해당하는 SchedulerService.class 는 말 그대로 클라이언트입니다. 그러나 코드를 통해 보면 클라이언트가 Repository 를 통해 값을 가져오기도 하고, 심지어 상태를 업데이트하기도 합니다. 진짜 큰 문제는 @Scheduled(cron = "0 * * * * *") 로 동작되는 runWithScheduled() 는 한가지 역할만 수행하지 않고 있습니다.

 

이 코드는 아래와 같이 변경됩니다.

public class SchedulerClient {

    private final NotificatorProcessor processor;

    public SchedulerClient(NotificatorProcessor processor) {
        this.processor = processor;
    }

    @Scheduled(cron = "0 * * * * *")
    public void runWithScheduled() {
        processor.execute();
    }
}

Client 이라는 것을 명확히 표현하고(이전에는 SchedulerSerivce 였는데, 명명하면서도 어색하다는 것을 느꼈습니다), Executor 는 Processor 라는 이름으로 변경되고, 이전에 이곳에서 여러 일어나던 코드가 이전하게 되었습니다.

 

클라어인트 입장에서는 processor.execute() 만 고려하고, 안에 어떤 행위가 이뤄질 것인가는 알 필요가 없어집니다.

 

객체지향프로그래밍이란 역할,책임,협력에 의해서 만들어진다고 수백번 입으로 말하고 손으로 표현하려고 합니다.

그러나, 여전히 백문이불여일타 입니다.