[리팩토링] 중복인 줄 알았던 코드가 사양이었다 — 차수 계산을 값 객체로 모으기

같은 보고서인데 화면엔 #17, 공식 출력물엔 #23. 버그인 줄 알고 달려들었다가, 절반은 사양이고 절반은 진짜 버그였던 이야기.

TL;DR

“비슷한데 미묘하게 다른” 중복 로직을 발견하면, 통일하기 전에 먼저 물어야 한다: 이 차이가 드리프트(방치된 발산)인가, 사양(다른 질문에 대한 다른 답)인가.
우리 시스템엔 보고서 차수를 계산하는 로직이 두 벌 있었고 결과도 서로 달랐다. 나는 드리프트로 판정했다. 틀렸다. 과거 CS 티켓을 파보니 고객이 명시적으로 요구한 서로 다른 두 규칙이었다.
진짜 문제는 규칙이 둘이라는 사실이 코드 어디에도 적혀 있지 않았다는 것. 그래서 나처럼 아는 사람이 없으면 누구든 오진하는 구조였다.
처방은 흩어진 계산을 값 객체 하나(FollowUpChains)로 모으고, 두 규칙을 이름 붙은 메서드 두 개로 박제하는 것. 정적 팩토리 + 불변으로 “완제품으로만 태어나게” 강제했다.
덤으로 진짜 버그 두 개(조용한 폴백, 삭제 데이터 카운트)가 이 과정에서 드러나고 제거됐다.

1. 발단 — 같은 보고서인데 번호가 다르다

내가 맡고 있는 시스템은 하나의 “사례(case)“에 후속 보고서가 계속 쌓이는 도메인이다. 최초 보고(Initial)가 하나 있고, 새 정보가 들어올 때마다 추적 보고(Follow-up)가 체인처럼 이어진다. 화면에는 각 보고서가 몇 차인지가 표시되고, 공식 서식으로 출력할 때도 차수가 찍힌다.

그런데 이상한 걸 발견했다. 같은 보고서가:

목록 화면에서는 Follow up #17
공식 출력물에서는 #23

버그 아닌가? 코드를 열었다.

2. 코드를 보니 — 계산 로직이 두 벌

차수를 계산하는 코드가 두 군데 있었다.

구현 1 — 조회 서비스 안의 공용 메서드 (화면·목록·엑셀 다운로드가 사용):

// 체인을 돌면서 삭제는 건너뛰고, FOLLOW_UP만 1부터 센다
int sequence = 0;
for (ReportSummary summary : chain) {
    if (summary.isDeleted()) continue;
    if (summary.getClassification() == FOLLOW_UP) {
        numberByReportId.put(summary.getId(), ++sequence);
    }
}

구현 2 — 출력물 빌더 안의 private 메서드:

// 체인 전체에서 자기 위치를 찾는다. 못 찾으면... 전체 건수?
private int getSequenceNumber(List<ChainItem> chain, Long currentId) {
    int number = chain.size();               // ← 이게 기본값이다
    for (int i = 0; i < chain.size(); i++) {
        if (chain.get(i).getId().equals(currentId)) {
            number = i + 1;
            break;
        }
    }
    return number;
}

체인을 DB에서 가져오는 방법(같은 사례번호로 묶고, 최신정보일 → 생성일 순 정렬)은 완전히 동일했다. 차이는 세는 방법에만 있었다:

	구현 1 (화면)	구현 2 (출력물)
Initial 보고	안 셈	한 자리 차지
삭제된 보고	제외	포함
자기를 못 찾으면	null	조용히 전체 건수

그래서 화면 #17이 출력물에선 #23이 됐던 것이다. 앞에 삭제된 보고 5건 + Initial 1건 = +6.

3. 오진 — 드리프트로 판정했다

파울러의 리팩토링을 읽고 있던 참이라, 이건 교과서에 나오는 최악의 중복으로 보였다. 똑같은 복붙은 눈에 띄기라도 하지, “비슷한데 다른” 중복은 어느 쪽이 버그고 어느 쪽이 의도인지 아무도 모르는 채로 늙어간다.

판정: 구현 2는 옛날에 따로 짠 코드가 방치되면서 발산한 드리프트다. 구현 1로 통일하자.

이때 배운 것 하나. 삼진 규칙(Rule of Three)이 세는 건 호출자 수가 아니라 로직 사본 수다. 공용 메서드 하나를 열 군데서 불러도 그건 중복이 아니다. 하지만 지금은 사본이 두 벌이고 서로 다르다 — 명백한 통일 대상이라고 확신했다.

4. 반전 — CS 티켓이 답을 갖고 있었다

통일하기 전에, 출력물 숫자는 고객에게 나가는 값이라 근거를 확인하기로 했다. 과거 CS 티켓을 뒤졌다.

그리고 뒤집혔다. 몇 달 전 고객이 정확히 이 차수에 대해 문의한 티켓이 있었는데, 고객의 기대값이 이랬다:

“추적 1차 보고서는 #2, 추적 2차 보고서는 #3으로 출력되어야 하지 않을지 문의드립니다.”

추적 1차가 #2라는 건 Initial이 #1을 차지하는 카운트를 원한다는 뜻이다. 즉 구현 2의 “+1”은 드리프트가 아니라 고객이 명시적으로 확정해준 사양이었다.

두 숫자는 같은 질문에 대한 다른 답이 아니라, 다른 질문에 대한 각자의 답이었던 것이다:

화면: “이건 몇 번째 추적 보고냐?” → 첫 추적 = #1
출력물: “이건 이 사례의 몇 번째 보고서냐?” → Initial = #1, 첫 추적 = #2

내 오진의 원인은 명확했다. 이 구분이 코드 어디에도 적혀 있지 않았다. 두 구현은 각자의 파일에서 아무 설명 없이 다르게 세고 있었고, 그 차이가 의도라는 증거는 코드 밖(CS 티켓)에만 존재했다. 암묵지가 코드 밖에 있으면, 다음 사람은 반드시 나처럼 오진한다.

5. 진짜 버그는 따로 있었다

사양을 확인하고 나니, 구현 2에서 사양이 아닌 것들이 선명하게 분리됐다.

버그 1 — 조용한 폴백. int number = chain.size() 이 한 줄. 체인에서 자기 id를 못 찾으면(동기화 지연, 식별자 불일치 등) 실패라고 말하는 대신 전체 건수를 돌려준다. 실제로 과거 CS의 원인이 이거였다 — 모든 차수가 “전부 최대값”으로 찍혀서 고객이 발견했다. 코드가 “나 실패했어”라고 말할 방법이 없으니, 실패가 그럴듯한 숫자로 위장한 것이다.

버그 2 — 삭제 데이터 카운트. 삭제된 보고는 화면에서도, 후속 보고 생성 로직에서도 전부 제외하는 게 시스템의 일관된 규칙인데 출력물만 셌다. 흥미로운 건 체인 DTO가 deleted 플래그를 들고 오는데 안 쓰고 있었다는 점 — 필터하려다 만 흔적처럼 보였다. 이것 때문에 삭제가 낀 사례는 출력 차수가 계속 부풀었다.

정리하면: 번호 매기는 기준은 요구사항이 맞았고, 구현 품질이 엉성했던 것. “이상해 보이는 코드”를 사양과 버그로 쪼개는 데 필요한 건 코드 읽기가 아니라 도메인 확인이었다.

6. 처방 — 값 객체로 모으고, 규칙에 이름을 붙인다

파울러 2판의 여러 함수를 클래스로 묶기(Combine Functions into Class)가 정확히 이 상황이다: 같은 데이터(체인)를 놓고 협력하는 함수 무리가 보이면 클래스로 묶는다.

/**
 * 사례번호로 묶인 보고 체인들 — 차수 산정 규칙의 단일 거처.
 *
 * 같은 사례의 "차수"에 답이 두 개인 건 질문이 다르기 때문이다:
 * - followUpNumberOf: 몇 번째 '추적' 보고냐. Initial 제외, 첫 추적이 1. (화면·목록·엑셀)
 * - reportNumberOf:   사례의 몇 번째 '보고서'냐. Initial이 1, 첫 추적이 2. (공식 출력물 — 고객 확정 사양)
 *
 * 삭제된 보고는 두 규칙 모두 제외한다.
 * 체인에 없는 id는 null — 호출부가 폴백을 결정한다(전체 건수 폴백 금지: 과거 CS의 원인).
 */
public class FollowUpChains {

    /** 체인 정렬: 최신정보일 → 생성일 오름차순. 정렬이 곧 차수 규칙의 일부다 */
    private static final Comparator<ReportSummary> CHAIN_ORDER = ...;

    private final Map<Long, Integer> followUpNumberById = new HashMap<>();
    private final Map<Long, Integer> reportNumberById = new HashMap<>();

    private FollowUpChains() {}

    /** 입력 순서 무관 — 체인 정렬은 여기서 직접 한다 */
    public static FollowUpChains of(List<ReportSummary> summaries) {
        FollowUpChains chains = new FollowUpChains();
        // 사례번호별로 묶고, 정렬하고, 삭제 제외하고, 두 규칙의 번호를 전부 계산해서 품고 태어난다
        ...
        return chains;
    }

    public Integer followUpNumberOf(Long reportId) { ... }
    public Integer reportNumberOf(Long reportId) { ... }
}

핵심은 두 가지다.

첫째, 규칙 두 개가 이름 붙은 메서드 두 개가 됐다. followUpNumberOf와 reportNumberOf. javadoc에 “왜 둘인지”와 근거(고객 확정 CS 티켓 번호)까지 박았다. 다음 사람은 — 6개월 뒤의 나를 포함해서 — 이 파일을 열면 오진할 수가 없다. 암묵지가 코드 안으로 들어왔다.

둘째, 소비자들이 계산을 잊었다. 리팩토링 전에는 “어떻게 세는지”가 서비스와 출력물 빌더에 각자 흩어져 있었다. 지금은 둘 다 계산법을 모른다:

// 서비스 — 조회하고, 만들고, 질문만 던진다
FollowUpChains chains = FollowUpChains.of(repository.findChainsBy(companyId, caseNumbers));
Integer number = chains.followUpNumberOf(reportId);

서비스 코드가 눈에 띄게 줄었고, 다음에 “차수 계산 관련 요구사항”이 오면 수정 지점은 한 곳이다. 파울러의 그 문장 그대로 — 변경을 쉽게 만든 다음, 쉬운 변경을 하라.

7. 강제성 — 완제품으로만 태어나게

이 클래스에는 의도적인 강제 장치가 네 개 있다.

1. 태어나는 입구가 하나다. 생성자가 private이고 of() 정적 팩토리로만 만들 수 있다. 이 객체는 빈 채로 존재하면 의미가 없다 — 반드시 “체인을 받아서 번호를 전부 계산해둔 상태”로 태어나야 한다. new를 막아두면 반쯤 조립된 객체가 돌아다니는 사고가 컴파일 단계에서 봉쇄된다. LocalDate.of(), List.of()가 쓰는 바로 그 패턴이다.

2. 태어난 뒤엔 못 바뀐다. setter가 없고 내부 맵은 final. 가능한 건 질문뿐이다.

3. 모르는 건 모른다고 답한다. 체인에 없으면 null. 호출부는 “그럼 어떡할지”를 강제로 결정해야 한다. 옛 코드의 size() 폴백은 정확히 이 강제가 없어서 난 사고였다 — 호출부가 아무 생각 없이 믿을 수 있는 그럴듯한 숫자를 줬으니까. 새 구조에선 산정 불가 시 출력물에 틀린 숫자 대신 공란이 나간다. 실패가 눈에 보인다.

4. 전제조건도 없다 — 정렬은 자기 책임. 고백하자면 이건 처음엔 없었다. 첫 버전의 of()는 javadoc에 “리스트는 체인 정렬 상태여야 한다 — 리포지토리 쿼리가 보장”이라고 적어두고 정렬된 입력을 믿었다. 강제성 얘기를 실컷 해놓고, 정작 제일 중요한 전제는 강제가 아니라 주석으로 부탁하고 있었던 것이다. 코드 리뷰에서 걸렸다: 지금이야 호출 경로가 하나라 안전하지만, 이 클래스가 성공해서 소비자가 늘수록 누군가 정렬 안 된 리스트를 넣는 날이 오고, 그러면 차수가 조용히 뒤섞인 채 계산된다 — 우리가 방금 죽인 size() 폴백과 같은 부류의, 소리 없는 오답이다. 생각해보면 정렬 기준(최신정보일 → 생성일)은 DB 사정이 아니라 “체인이란 무엇인가”라는 도메인 지식이다. 규칙의 단일 거처라면서 정렬 규칙만 리포지토리에 두는 건 반쪽이었다. Comparator 상수로 끌어와 of() 안에서 직접 정렬하는 걸로 수정 — 이제 이 객체는 입력에 대해 아무것도 요구하지 않는다.

김영한 님이 강의에서 “약간의 강제성이 있어야 좋은 프로그램”이라고 했는데, 이게 그 말의 실체라고 생각한다. 문서로 “이렇게 써주세요” 부탁하는 게 아니라, 잘못 쓰는 코드가 컴파일되지 않게 만드는 것. 함수형 진영의 격언으로는 make illegal states unrepresentable.

테스트는 값 객체라 Spring 없이 순수하게 돌아간다. @DisplayName으로 규칙을 문장으로 적어두면 테스트가 곧 동작 문서가 된다:

@Test
@DisplayName("공식 출력물 차수는 Initial이 1을 차지해 첫 추적 보고가 2다")
void reportNumberCountsInitialAsOne() { ... }

@Test
@DisplayName("체인에 없는 보고는 null이다 — 전체 건수 폴백 없음(과거 CS 재발 방지)")
void returnsNullForUnknownIdInsteadOfChainSize() { ... }

8. 배운 것

“비슷한데 다른” 중복은 통일 전에 도메인에 물어라. 코드만 보고 어느 쪽이 정답인지 정할 수 없다. 나는 코드 읽기로 드리프트라 확신했고, CS 티켓 하나에 뒤집혔다. 수정이 쉬운 것과 수정해도 되는 것은 다른 문제다.
삼진 규칙은 호출자 수가 아니라 로직 사본 수를 센다. 공용 함수 하나를 여러 곳에서 부르는 건 건강한 상태다.
규칙이 둘이면, 둘이라는 사실 자체를 코드에 적어라. 이번 리팩토링의 진짜 배당금은 코드 축소가 아니라 “다른 질문 두 개”라는 암묵지가 이름 붙은 메서드 두 개로 박제된 것이다.
조용히 그럴듯한 값을 돌려주는 코드가 제일 위험하다. 시끄러운 실패는 개발자가 발견하지만, 조용한 오답은 고객이 발견한다.
값 객체 추출은 정적 팩토리·불변과 세트로 온다. 계산을 생성 시점에 끝내려면 생성 입구를 통제해야 하고, 통제하고 나면 불변이 공짜로 따라온다.
원칙은 자기 코드에서 먼저 샌다. 강제성을 설계 원칙으로 내세운 바로 그 클래스에서, 정렬 전제를 주석으로 부탁하고 있었다. “주석으로 부탁하는 전제가 남아 있나?”는 리뷰 때마다 던질 가치가 있는 질문이다.

···