리팩토링 19. 질의 함수와 변경 함수 분리하기

들어가기 전

이 글은 인프런 백기선님의 강의를 복습하며 작성한 글입니다. 

---

리팩토링 19. 질의 함수와 변경 함수 분리하기 (Separate Query from Modifier)

• '눈에 띌만한 사이드 이펙트' 없이 값을 조회할 수 있는 메서드는 테스트 하기 쉽고, 메서드를 이동하기도 편하다.
• 명령-조회 분리 (command-query separation) 규칙 
  • 어떤 값을 리턴하는 함수는 사이드 이팩트가 없어야 한다.
  • 수정하는 메서드는 리턴값이 void여야만 한다.
• '눈에 띌만한 사이드 이펙트'
  • 가령, 캐시는 중요한 객체 상태 변화는 아니다. 따라서 어떤 메서드 호출로 인해, 캐시 데이터를 변경하더라도 분리할 필요는 없다. 
  • 객체의 상태가 변하는 것이 눈에 띌만한 사이드 이펙트임.

---

명령-조회 분리

일반적으로 Query / Modifier의 의미는 다음과 같다

• Query : 조회하는 함수를 의미.
• Modifier : 객체의 상태/값을 변경. 다른 행위(알림을 보냄)를 일으킬 수 있는 함수를 의미 (눈에 띌만한 사이드 이펙트)

명령(Modifier)-조회(Query)를 각각 분리하자는 것은 값을 조회하는 함수 / 값을 변경하는 함수를 구분해서 조회 시, 사이드 이펙트가 발생할 것을 줄이자는 것이다. 

값을 조회하는 함수는 사이드 이펙트가 없어야 한다. 예를 들어 값을 부가적으로 출력한다거나, 객체의 상태를 변경한다거나 하는 것을 없애야 한다. 눈에 띌만한 사이드 이펙트가 없는 메서드는 테스트 하기도 편하고, 메서드를 다른 곳으로 이동하기도 편리하기 때문이다. 사용하는 입장에서도 사이드 이펙트 없는 메서드가 사용하기 편리하다. 만약 조회할 때 내부적으로 값이 변경된다면 더욱 고려해야 할 부분이 많아지기 때문이다. 

---

눈에 띌만한 사이드 이펙트

아주 분명하게 객체의 상태가 변하는 것을 의미한다. 예를 들면 사용자에게 메일을 보내거나, 객체의 값이 바뀌는 것이다.그런데 '캐시'가 변하는 경우는 '눈에 띌만한 사이드 이펙트'로 생각하지 않는다. 캐시는 조회한 값을 저장하는 역할을 하기 때문에 조회 시, 캐시가 변할 수 있다. 그런데 저자는 캐시 같은 경우는 '눈에 띌만한 사이드 이펙트'가 아니라고 생각한다.

---

실습1

getTotalOutstandingAndSendBill() 메서드를 보자. 이 메서드는 두 가지 일을 한다.

• Outstanding invoice의 total 값을 계산한다. (단순 조회)
• 영수증을 보낸다.  (변경)

나는 total이 얼마인지만 보고 싶은데, 손님에게 영수증을 계속 보낸다.

• '얼마인지만 보고 싶다' = 조회
• 손님에게 영수증을 보낸다 = 눈에 띌만한 사이드 이펙트

// Total 값을 조회 + 영수증 전송 --> 조회 + 수정을 동시에 하는 중. 냄새 분리 필요.
public double getTotalOutstandingAndSendBill() {
    double result = customer.getInvoices().stream()
            .map(Invoice::getAmount)
            .reduce((double) 0, Double::sum);
    sendBill();
    return result;
}

따라서 이 두 개의 행위를 분리하는 것이 더욱 좋다. 리팩토링을 하면서 아래와 같이 getTotalOutStanding() / sendBill() 메서드로 분리해준다. 

// Total 값을 조회 + 영수증 전송 --> 조회 + 수정을 동시에 하는 중. 냄새 분리 필요.\
// 조회 전용 메서드
public double getTotalOutstanding() {
    return customer.getInvoices().stream()
            .map(Invoice::getAmount)
            .reduce((double) 0, Double::sum);
}

// '수정' 메서드는 void를 리턴한다.
// 수정 전용 메서드
public void sendBill() {
    emailGateway.send(formatBill(customer));
}

---

 

실습2.

alertForMiscreant() 메서드를 살펴보자. 이 메서드는 두 가지 작업을 하고 있다. 

• 해당되는 사람이 있는지 조회하는 로직 (조회)
• 해당되는 사람의 알람을 끄는 로직  (명령)

조회와 업데이트가 동시에 진행되고 있는데, 이 부분을 '명령-조회 분리'로 처리가 필요하다. 

public class Criminal {

    // 1. Don, John인지를 조회
    // 2. 맞는 경우 알람 끄기
    // 조회 + 업데이트 두 가지 작업을 하는 중 → 명령 + 조회로 분리가 필요함. 
    public String alertForMiscreant(List<Person> people) {
        for (Person p : people) {
            if (p.getName().equals("Don")) {
                setOffAlarms();
                return "Don";
            }

            if (p.getName().equals("John")) {
                setOffAlarms();
                return "John";
            }
        }

        return "";
    }

    private void setOffAlarms() {
        System.out.println("set off alarm");
    }
}

다음과 같이 두 가지로 분리했다.

1. 해당되는 사람이 있는지 조회하는 쿼리 메서드
2. 해당되는 사람이 있는 경우, 알람을 끄는 명령 메서드 

유의해야 할 부분은 조회 메서드는 특정 값을 반환하도록 하고, 업데이트를 하는 메서드는 어떠한 값도 반환하지 않도록 void 타입을 설정했다. 

public class Criminal {

    // 1. Don, John인지를 조회
    // 2. 맞는 경우 알람 끄기
    // 조회 + 업데이트 두 가지 작업을 하는 중 → 명령 + 조회로 분리가 필요함.
    public void alertForMiscreant(List<Person> people) {
        for (Person p : people) {
            if (p.getName().equals("Don")) {
                setOffAlarms();
            }

            if (p.getName().equals("John")) {
                setOffAlarms();
            }
        }
    }
    
    public String findMiscreant(List<Person> people) {
        for (Person p : people) {
            if (p.getName().equals("Don")) {
                return "Don";
            }

            if (p.getName().equals("John")) {
                return "John";
            }
        }

        return "";
    }

    private void setOffAlarms() {
        System.out.println("set off alarm");
    }
}

---

실습3. 왜 조회 + 명령이 있는 메서드는 테스트 하기 어려울까?

alertForMiscreant() 메서드가 리팩토링 되기 전의 메서드라고 가정해보자. 즉, 조회 + 명령이 함께 있는 메서드다. 이 메서드는 왜 테스트 하기 어려울까? 

@Test
void alertForMiscreant() {
    Criminal criminal = new Criminal();
    String found = criminal.alertForMiscreant(List.of(new Person("Keesun"), new Person("Don")));
    assertEquals("Don", found);

    found = criminal.alertForMiscreant(List.of(new Person("John"), new Person("Don")));
    assertEquals("John", found);

    found = criminal.alertForMiscreant(List.of(new Person("Keesun"), new Person("Whiteship")));
    assertEquals("", found);
}

위 코드를 테스트 한 결과는 아래처럼 나온다. 살펴보면 3개의 메서드가 실행되었고, 2개의 메서드만 성공해서 'set off alram'이 2번 울리는 것을 확인할 수 있다. 그런데 왜 테스트가 어렵다고 하는 것일까?

그 이유는 다음과 같다.

어떤 변수가 주어졌을 때, set off alarm이 울렸는지 알 수 없다. 

단순히 조회만 하는 것이라면 assertEquals() 메서드에서 테스트의 통과 / 실패를 확인할 수 있다. 그렇지만 메서드 내부에서 상태를 변경 및 출력하는 '명령' 계통의 성향도 함께 존재하기 때문에 어떤 메서드에서 'set off alarm'이 출력되지 않은 것인지 알 수 없다. 

만약 테스트 결과가 set off alram이 3개 다 출력되어야 하는 상황인데, 2개만 울렸다면 하나씩 디버깅을 찍어가면서 어디에서 문제가 발생했는지 살펴봐야한다. 가장 무서운 것은 일단은 이 테스트가 통과한다는 것이고, 발생하는 사이드 이펙트가 우리가 기대하는 것과 다르다는 것이다. 그리고 그 사이드 이펙트의 정합성 문제는 이 테스트가 '신뢰할만한 테스트'인지를 다시 한번 고민하게 만들 것이다.