환불 처리 코드에 눈에 거슬리는 줄이 있었다.
if (SOME_BRANDS.contains(brand)) { // 즉석에서 만든 리스트, 호출마다 재생성
return refundHolder;
} else {
externalApi.transferCancelAndRefund(refundPayment, targetPayment, merchantInfo);
}
호출마다 리스트를 새로 만들고 contains는 O(n)이다. 상수 Set으로 빼는 건 당연한 정리였다. 그런데 이 줄을 들여다보다 더 큰 게 보였다. if의 return refundHolder와 메서드 끝의 return refundHolder는 같은 값이었다. 두 분기의 유일한 차이는 외부 거래전송 호출 여부뿐. 그래서 조기 return을 없애고 부정 조건 하나로 줄였다.
여기까지는 몸풀기였다. 진짜 요청은 그다음이었다: "환불이라는 행위는 전략이 있든 없든 공통으로 처리돼야 한다."
후처리가 한쪽 분기에만 있었다
환불에는 두 갈래가 있다. 전략 패턴을 쓰는 결제사는 전략 경로로, 나머지는 일반 경로로 간다. 문제는 완료 게이트 + 실시간 이벤트 발행 + 외부 거래전송이라는 후처리가 일반 경로(else) 안에만 있었다는 것이다. 전략 경로는 환불 결과와 무관하게 그냥 return했다.
요청은 명확했다. 후처리를 else 밖으로 끌어올려 두 경로가 공통으로 타게 하자. "빼기만 하면 되겠네" 싶었다.
함정 1 — 순진하게 빼면 새 외부 호출이 샌다
빼기 전에 한 가지를 확인했다. 어떤 결제사가 전략 경로인가. 그리고 방금 만든 "거래전송 제외" 집합에는 그 전략 경로 결제사들이 들어있지 않았다.
즉 후처리를 그대로 밖으로 빼면, 지금까지 외부 거래전송을 한 번도 하지 않던 결제사들이 그 API를 새로 호출하게 된다. 이건 리팩터링이 아니다. 외부 시스템으로 나가는 신규 side effect다. 결제에서 "동작은 그대로"라는 약속을 깨는 순간, 그건 리팩터링이 아니라 배포 사고 후보가 된다.
앞선 다른 작업에서 얻은 교훈의 정확한 역상이었다 — 출력에서 안 쓰는 필드를 지우면 상류가 dead code로 죽는다. 반대로, 한쪽 분기에만 있던 로직을 공통으로 끌어올리면 없던 동작이 새어 나온다. 하이스팅은 방향만 반대일 뿐 같은 위험이다.
함정 2 — 막으려던 코드가 컴파일되지 않았다
동작을 보존하려면 전략 경로 결제사를 제외 집합에 넣으면 된다. 간단해 보였다.
Set.of(/* 기존 제외 브랜드들 */, STRATEGY_BRAND_A, STRATEGY_BRAND_B);
컴파일이 깨졌다.
error: cannot find symbol
symbol: variable STRATEGY_BRAND_A
이 프로젝트는 사내 아티팩트 저장소의 공용 라이브러리에 의존하는데, 그 라이브러리의 소스 스냅샷에는 있는 enum 상수가, 실제 빌드가 당겨오는 배포된 jar에는 아직 없었다. 눈에 보이는 코드와 빌드가 참조하는 바이너리가 어긋나 있었던 것이다. 소스만 믿고 심볼을 쓰면 로컬에서 막힌다.
결정 — 대상을 열거하지 말고, 경로로 판단한다
함정 2는 우회할 수도 있었다. 하지만 그게 오히려 더 나은 설계를 밀어줬다.
제외해야 할 대상을 브랜드 이름으로 열거하는 대신, "전략 경로면 외부 거래전송을 하지 않는다" 는 규칙으로 표현하면 —
- 기존 동작과 정확히 동일하다. 거래전송은 원래부터 일반 경로에서만 일어났으니까.
- 배포 jar에 없는 상수에 의존하지 않는다.
- 브랜드를 나열하지 않으니, 앞으로 전략 결제사가 늘어도 자동으로 반영된다. 열거 방식은 추가할 때마다 집합을 갱신해야 하고, 빠뜨리면 조용히 신규 거래전송이 나간다.
트레이드오프는 있다. 열거는 "무엇을 제외하는가"가 눈에 보이고, 술어는 "전략 = 거래전송 안 함"이라는 규칙을 코드로 못박는다. 지금은 그 규칙이 사실이므로 술어가 맞다. 미래에 "전략인데 거래전송도 필요한" 결제사가 생기면 그때 다시 명시적 정책이 필요할 것이다 — 그건 그때의 문제다.
해결
refund()는 어느 경로를 탔는지만 기억하고, 후처리는 공통 메서드 하나에 맡긴다.
boolean strategyHandled = strategyFactory.isSupported(company.getCode());
RefundHolder refundHolder = strategyHandled
? processWithStrategy(...)
: processDefault(...);
// 전략/일반 경로와 무관하게 수행하는 환불 공통 후처리
completeRefund(refundPayment, targetPayment, merchantInfo, refundHolder, strategyHandled);
return refundHolder;
void completeRefund(..., boolean strategyHandled) {
if (!isCompleted(refundPayment)) return;
publishRealtimeEvents(refundPayment, refundHolder); // 실시간 이벤트는 공통
if (strategyHandled) return; // 전략 경로는 거래전송 대상 아님
if (!TRANSFER_EXCLUDED.contains(brandOf(refundPayment))) {
externalApi.transferCancelAndRefund(refundPayment, targetPayment, merchantInfo);
}
}
하이스팅에는 뒷정리가 딸려 왔다. 완료 게이트를 공통 메서드로 올렸으니, 전에 전략 경로 안에 넣어뒀던 같은 게이트는 제거해야 했다. 안 그러면 전략 경로에서 실시간 이벤트가 두 번 발행된다. 게이트가 한 곳에만 있도록 정리하고, 테스트도 completeRefund 기준으로 다시 썼다 — 일반 경로 완료 시 거래전송 수행, 제외 대상은 미수행, 전략 경로는 미수행(동작 보존 회귀 방지), 미완료면 아무것도 안 함.
아쉬운 점, 그리고 다음
- 테스트를 로컬에서 돌리지 못했다. 함정 2와 같은 이유로 테스트 컴파일이 막혀 있어, 본 소스 컴파일 통과까지만 확인하고 실행 검증은 다음 단계로 넘겼다. 근본 해결은 코드가 아니라 공용 라이브러리를 최신으로 빌드해 사내 저장소에 재배포하는 파이프라인이다. 그게 없는 한 이 함정은 계속 재발한다.
- 술어의 유효기간. "전략 = 거래전송 안 함"은 오늘의 사실이지, 영원한 규칙이 아니다. 이 가정이 깨지는 순간을 대비해 결정 근거를 주석과 이 글에 남겨 뒀다.
- 이름이 뉘앙스를 다 담지 못한다. 제외 집합의 이름은 "비전략 경로 중 제외 대상"만 의미하는데, 이름만 보면 전략 결제사도 포함일 것 같다. 경로 분기가 이름 밖으로 새어 있는 셈이다.
가장 오래 남을 교훈은 이것이다. "공통으로 빼자"는 리팩터링처럼 들리지만, 한쪽에만 있던 로직에는 한쪽에만 있어야 할 이유가 숨어 있을 수 있다. 빼기 전에 "이걸 안 타던 쪽은 왜 안 탔나"를 먼저 물어야 한다. 그 질문이 두 함정을 다 잡았다.
