소프트웨어 공학 · 심화
코드 리뷰 실무 - 버그를 잡고 팀을 키우는 읽기
설계 냄새 리뷰 - 책임·중복·의존
너무 많은 일을 하는 클래스, 같은 규칙의 중복, 구체 클래스에 묶인 의존, 이름이 말하지 않는 의도, 리뷰에서 지적할 범위와 다음 PR 로 넘길 범위, 완성 코드는 중복된 요금 규칙을 하나로 모은 전/후의 결과가 같은지 확인한다
개발자KR · 원고 갱신
이 장에서 배우는 것
대출 서비스의 계산 결과가 맞아도, 규칙 하나를 바꾸려고 여러 메서드를 찾아다녀야 한다면 다음 변경에서 실수가 생기기 쉽다. 설계 냄새(design smell)는 이런 변경의 어려움을 의심하게 하는 단서다. 이름이 마음에 들지 않거나 클래스가 길다는 이유만으로 결함이라고 단정하는 기준은 아니다. 리뷰에서는 현재 변경이 어떤 책임과 규칙에 걸쳐 있는지 살피고, 실제로 수정할 때 놓칠 수 있는 지점을 근거로 설명해야 한다.
앞 장에서 예외가 실패의 이유를 전달하는 방법을 살펴보았다. 여기서는 정상적으로 실행되는 코드 안에서 책임, 중복, 의존이 변경을 어렵게 만드는 모습을 읽는다. 도서관 대출 서비스의 중복된 요금 규칙을 한곳으로 모으되, 같은 입력에 대한 계산 결과와 안내 문구가 유지되는지 실행으로 확인한다.
- 너무 많은 일을 하는 클래스를 줄 수보다 변경 이유로 판단한다.
- 비슷한 코드와 같은 업무 규칙의 중복을 구별한다.
- 구체 클래스에 묶인 의존을 발견하고, 필요한 동작만 받도록 바꾼다.
- 이름으로 입력의 의미와 메서드의 결과를 드러낸다.
- 이번 변경에서 고칠 범위를 정하고, 전후 동작을 비교한다.
문제 상황
도서관 대출 서비스에는 예상 연체료를 보여 주는 화면과 반납 안내 문구를 만드는 기능이 있다. 처음에는 두 기능을 같은 개발자가 만들었다. 예상 금액을 계산하는 코드를 안내 문구 메서드에 복사해 넣었고, 두 메서드는 한동안 같은 결과를 냈다.
이번 변경 요청은 안내 문구에도 예상 금액과 같은 규칙을 적용하도록 계산 경로를 정리하는 일이다. 팀이 예제로 정한 규칙은 연체 일수 중 처음 2일을 계산에서 빼고, 나머지 하루마다 100원을 더하되 최대 500원까지만 받는 것이다. 이 수치는 실제 기관의 정책이 아니라 이 장의 동작 비교를 위한 고정 규칙이다. 입력인 연체 일수는 이미 확정된 0 이상의 정수로 가정한다.
수정 전 서비스는 대출 정보 조회, 요금 계산, 안내 문구 생성을 모두 맡는다. 데이터는 메모리에 있지만, 서비스가 그 저장 클래스를 직접 생성한다. 작성자는 두 계산식 중 하나만 고치면 다른 화면에 다른 금액이 나타날 수 있다는 점을 발견하고 리뷰를 요청했다.
예상 금액과 안내 문구가 같은 요금 규칙을 각각 구현하고 있다. 규칙 변경 시 두 곳을 함께 고쳐야 하므로, 이번 변경에서는 계산을 한곳으로 모으고 두 경로의 결과를 함께 확인하면 좋겠다.
이 의견은 단순히 “중복을 없애자”라고 말하지 않는다. 중복된 것이 무엇인지, 어떤 변경에서 문제가 생기는지, 어디까지 확인할지 연결한다. 리뷰 대상은 코드의 모양보다 변경의 위험이다.
책임은 클래스의 길이보다 변경 이유로 읽는다
책임은 클래스가 가진 메서드 이름을 나열하는 것만으로 드러나지 않는다. 어떤 요청 때문에 그 클래스를 수정하게 되는지 묻는 편이 유용하다. 대출 정보를 읽는 방법이 바뀔 때, 요금 정책이 바뀔 때, 안내 문장의 표현이 바뀔 때 모두 같은 클래스를 열어야 한다면 서로 다른 변경 이유가 모여 있는 것이다.
그렇다고 변경 이유가 둘 이상이라는 이유만으로 곧바로 클래스를 나눌 필요는 없다. 서비스는 여러 동작을 순서대로 연결하는 역할을 맡을 수 있다. 대출을 조회한 뒤 요금을 계산하고 문구를 만드는 흐름 자체는 하나의 사용 목적에 속한다. 문제는 그 흐름을 연결하는 코드가 조회 방식과 계산식의 세부 사항까지 모두 알고 있을 때 커진다.
이번에는 서비스가 “대출을 찾고, 계산을 요청하고, 결과를 문구에 넣는다”라는 흐름을 유지한다. 요금 계산식은 별도 클래스로 옮기고, 조회는 필요한 동작만 표현한 인터페이스로 받는다. 안내 문구가 짧고 변형도 하나뿐이므로 문구 전용 클래스까지 만들지는 않는다. 책임 분리는 가능한 클래스를 모두 만드는 작업이 아니다.
분리 뒤에도 서비스가 계산 객체를 호출한다는 사실은 남는다. 의존을 없앤 것이 아니라, 무엇을 알아야 하는지 좁힌 것이다. 서비스가 알아야 할 것은 연체료를 얻는 방법이지, 유예 일수와 상한 금액을 적용하는 계산 순서가 아니다.
| 관찰한 코드 | 확인할 변경 이유 | 이번 판단 |
|---|---|---|
| 서비스가 저장 객체를 생성한다 | 조회 구현을 바꿀 때 서비스도 고치는가 | 조회 동작을 생성자로 받는다 |
| 두 메서드에 같은 계산식이 있다 | 요금 정책 변경이 두 곳에 퍼지는가 | 계산을 한곳으로 모은다 |
| 서비스가 짧은 문구를 조립한다 | 문구가 여러 형식으로 확장되었는가 | 현재는 서비스에 둔다 |
리뷰에서는 “이 클래스는 책임이 너무 많다”라는 평가에서 멈추지 않는다. “저장 방식 변경과 요금 정책 변경이 같은 클래스를 수정하게 만든다”처럼 구체적인 변경을 제시한다. 그래야 작성자가 분리의 이점과 추가 코드의 비용을 함께 판단할 수 있다.
같은 규칙의 중복과 구체적인 의존을 구별한다
중복은 같은 이유로 함께 바뀌는지를 묻는다
코드 두 줄이 닮았다는 사실과 업무 규칙이 중복되었다는 사실은 다르다. 서로 다른 정책이 우연히 같은 곱셈을 사용할 수 있다. 반대로 표현이 달라도 같은 요금 규칙을 두 번 구현할 수 있다. 한쪽은 조건문을 쓰고 다른 쪽은 최솟값과 최댓값으로 계산하더라도, 정책이 바뀔 때 둘을 함께 고쳐야 한다면 규칙의 중복이다.
이번 두 기능은 같은 대출의 같은 연체료를 보여 준다. 유예 일수, 하루 요금, 상한이 모두 함께 바뀌어야 한다. 따라서 계산을 하나로 모을 근거가 충분하다. 숫자를 상수로만 바꾸고 계산식을 두 군데 남겨 두면, 계산 순서나 상한 적용 방식을 수정할 때 여전히 둘을 고쳐야 한다.
분리한 계산 메서드는 연체 일수를 받아 금액을 반환한다. 저장 객체나 안내 문구를 입력으로 받지 않는다. 입력과 결과가 좁아지면 무엇을 확인해야 하는지도 선명해진다. 이 예제에서는 서비스의 두 경로가 이 메서드에 도달하도록 만드는 것이 핵심이다.
인터페이스는 서비스가 요구하는 동작을 표현한다
수정 전 서비스는 메모리 저장 클래스를 직접 만든다. 그 결과 다른 조회 방법을 사용하려면 서비스 안의 생성 코드를 바꿔야 한다. 수정 후에는 대출 하나를 찾는 동작만 가진 인터페이스를 받는다. 실행을 시작하는 main 메서드가 실제 저장 객체를 만들고 서비스에 전달한다.
이처럼 필요한 객체를 밖에서 받는 방식을 의존성 주입(dependency injection)이라고 한다. 별도 도구는 필요하지 않다. 생성자 매개변수로 전달하는 것만으로도 서비스와 객체 생성 위치를 분리할 수 있다. 다만 매개변수의 형식을 여전히 메모리 저장 클래스로 선언하면, 생성 위치만 옮겼을 뿐 구체 클래스에 대한 의존은 남는다.
모든 협력 객체에 인터페이스가 필요한 것은 아니다. 이번에는 조회 방식을 바꿔 끼우는 목적이 있으므로 조회에 인터페이스를 둔다. 요금 계산에는 현재 구현이 하나이고 다른 정책을 선택할 요구도 없으므로 구체적인 계산 클래스를 그대로 받는다. 추상화는 실제로 분리하려는 경계를 설명해야 한다.
이름은 숫자의 의미와 결과를 말해야 한다
계산 메서드가 run이고 입력이 d라면 호출부를 읽는 사람이 구현을 열어 의미를 복원해야 한다. feeForOverdueDays와 overdueDays는 입력이 연체 일수이고 결과가 요금이라는 뜻을 전달한다. 금액 변수에 원 단위를 나타내는 Won을 붙인 것도 서로 다른 단위의 숫자를 구별하기 위해서다.
이름이 모든 계약을 담을 수는 없다. 0 이상의 입력만 받는다는 전제나 상한의 구체적인 값은 설명과 확인 코드가 보완한다. 이름을 지나치게 길게 늘리는 대신, 같은 문맥에서 여러 뜻으로 읽히는 단어를 먼저 줄이는 편이 낫다.
리뷰의 범위는 확인할 수 있는 변경으로 정한다
설계 리뷰에서는 주변의 개선점이 연달아 보인다. 요금 계산을 옮기다 보면 안내 문구의 형식, 대출 정보의 저장 방식, 정책 선택 기능까지 손대고 싶어진다. 하지만 변경이 넓어지면 결과가 달라졌을 때 원인을 찾기 어렵고, 리뷰어가 따라가야 하는 가정도 늘어난다.
이번 변경의 중심은 중복된 요금 계산을 하나로 모으는 일이다. 조회 인터페이스는 같은 작업에서 서비스가 요구하는 동작을 드러내고, 서비스의 계산 경로를 독립적으로 확인하는 데 사용한다. 데이터 저장 형식을 바꾸거나 안내 문장을 수정하지는 않는다. 계산식의 이동과 출력의 변경을 한꺼번에 섞으면 동작 보존을 확인하기 어려워지기 때문이다.
이번에는 두 요금 계산을 하나로 모으고 기존 안내 문구가 유지되는지 확인하자. 안내 문구가 여러 형식으로 늘어날 때 전용 객체로 나누는 안은 다음 변경 후보로 남기면 좋겠다.
다음 변경으로 넘긴다는 말에는 다시 검토할 계기가 있어야 한다. “나중에 정리”라고만 적으면 판단 근거가 사라진다. “문구 형식이 추가될 때 분리 검토”처럼 관찰 가능한 조건을 남기면 이후 리뷰에서 같은 논의를 처음부터 반복하지 않아도 된다.
수정 전후가 같다는 확인만으로 계산이 옳다고 결론 내릴 수도 없다. 두 구현이 같은 실수를 공유할 수 있기 때문이다. 따라서 완성 코드는 두 가지를 확인한다. 먼저 수정 전후의 예상 금액과 안내 문구를 비교한다. 이어서 예제로 정한 규칙의 기대 금액과 수정 후 결과를 비교한다. 앞의 확인은 동작 보존을, 뒤의 확인은 선택한 사례에서 규칙이 맞는지를 다룬다.
아래 프로그램에는 비교를 위해 수정 전 클래스를 남겨 둔다. 실제 서비스에서 두 구현을 계속 운영하자는 뜻은 아니다. 변경을 검토하는 동안 전후 비교에 사용하고, 통합 이후에는 유지할 구현과 확인 사례를 중심으로 코드를 정리할 수 있다.
완성 코드
한 파일로 저장한다. 대출 정보는 수정 전후가 같은 조회 객체를 사용하며, 현재 시각이나 외부 저장소를 읽지 않는다. 모든 확인이 끝나면 프로그램은 바로 종료한다. 기록형인 record는 대출 정보와 비교 사례의 값을 담는 데 사용한다.
Main.java
import java.util.Map;
public class Main {
record Loan(String id, int overdueDays) {
}
record FeeCase(String id, int overdueDays, long expectedWon) {
}
interface LoanLookup {
Loan find(String loanId);
}
static final class MemoryLoanLookup implements LoanLookup {
private final Map<String, Loan> loans;
MemoryLoanLookup(Map<String, Loan> loans) {
this.loans = Map.copyOf(loans);
}
@Override
public Loan find(String loanId) {
Loan loan = loans.get(loanId);
if (loan == null) {
throw new IllegalArgumentException("대출 없음: " + loanId);
}
return loan;
}
}
static final class BeforeLoanService {
private final MemoryLoanLookup lookup;
BeforeLoanService(Map<String, Loan> loans) {
this.lookup = new MemoryLoanLookup(loans);
}
long quotedFeeWon(String loanId) {
int overdueDays = lookup.find(loanId).overdueDays();
int chargedDays = Math.max(0, overdueDays - 2);
return Math.min(500L, chargedDays * 100L);
}
String returnNotice(String loanId) {
int overdueDays = lookup.find(loanId).overdueDays();
int chargedDays = Math.max(0, overdueDays - 2);
long feeWon = Math.min(500L, chargedDays * 100L);
return "대출 " + loanId + ": 연체료 " + feeWon + "원";
}
}
static final class OverdueFeePolicy {
private static final int GRACE_DAYS = 2;
private static final long DAILY_FEE_WON = 100L;
private static final long MAX_FEE_WON = 500L;
long feeForOverdueDays(int overdueDays) {
int chargedDays = Math.max(0, overdueDays - GRACE_DAYS);
return Math.min(MAX_FEE_WON, chargedDays * DAILY_FEE_WON);
}
}
static final class LoanService {
private final LoanLookup lookup;
private final OverdueFeePolicy feePolicy;
LoanService(LoanLookup lookup, OverdueFeePolicy feePolicy) {
this.lookup = lookup;
this.feePolicy = feePolicy;
}
long quotedFeeWon(String loanId) {
Loan loan = lookup.find(loanId);
return feePolicy.feeForOverdueDays(loan.overdueDays());
}
String returnNotice(String loanId) {
long feeWon = quotedFeeWon(loanId);
return "대출 " + loanId + ": 연체료 " + feeWon + "원";
}
}
private static int failures;
private static void check(boolean condition, String name) {
System.out.println((condition ? "PASS " : "FAIL ") + name);
if (!condition) {
failures++;
}
}
public static void main(String[] args) {
FeeCase[] cases = {
new FeeCase("L0", 0, 0L),
new FeeCase("L2", 2, 0L),
new FeeCase("L3", 3, 100L),
new FeeCase("L6", 6, 400L),
new FeeCase("L7", 7, 500L),
new FeeCase("L9", 9, 500L)
};
Map<String, Loan> loans = Map.of(
"L0", new Loan("L0", 0),
"L2", new Loan("L2", 2),
"L3", new Loan("L3", 3),
"L6", new Loan("L6", 6),
"L7", new Loan("L7", 7),
"L9", new Loan("L9", 9)
);
BeforeLoanService before = new BeforeLoanService(loans);
LoanService after = new LoanService(
new MemoryLoanLookup(loans),
new OverdueFeePolicy()
);
for (FeeCase feeCase : cases) {
String loanId = feeCase.id();
check(
before.quotedFeeWon(loanId) == after.quotedFeeWon(loanId)
&& before.returnNotice(loanId)
.equals(after.returnNotice(loanId)),
"전후 동일: " + feeCase.overdueDays() + "일"
);
check(
after.quotedFeeWon(loanId) == feeCase.expectedWon(),
"규칙 기대값: " + feeCase.overdueDays() + "일"
);
}
LoanLookup fixedLookup = loanId -> new Loan(loanId, 3);
LoanService isolated = new LoanService(
fixedLookup,
new OverdueFeePolicy()
);
check(
isolated.returnNotice("LX").equals("대출 LX: 연체료 100원"),
"조회 구현 교체"
);
System.out.println(after.returnNotice("L9"));
System.out.println("실패 수: " + failures);
if (failures != 0) {
throw new AssertionError("확인 실패");
}
}
}줄별 해설
Loan과 FeeCase 선언은 업무 입력과 확인 기준을 구별한다. Loan은 대출의 연체 일수를 담고, FeeCase는 실행할 사례와 기대 금액을 담는다. 기대값을 계산 메서드로 다시 만들지 않고 직접 적어 둔 이유는 같은 계산 실수를 확인 코드에 복사하지 않기 위해서다.
LoanLookup의 find 선언은 서비스가 요구하는 조회 동작을 한 줄로 표현한다. 저장 방식 전체를 추상화하거나 저장소의 모든 기능을 옮기지 않는다. 이번 서비스가 사용하는 동작이 대출 하나를 찾는 것뿐이므로 인터페이스도 그 범위만 가진다.
MemoryLoanLookup의 생성자와 find는 메모리 조회의 세부 사항을 담당한다. 전달받은 맵은 복사해 보관하며, 없는 대출은 기존 방식대로 예외로 알린다. 이번 설계 변경에서는 없는 대출에 대한 계약을 바꾸지 않는다. 동작 보존 확인의 대상은 등록된 대출의 금액과 안내 문구다.
BeforeLoanService의 생성자는 저장 객체를 직접 생성한다. 이 부분이 구체 클래스에 묶인 의존을 보여 준다. quotedFeeWon과 returnNotice에는 같은 chargedDays 계산과 상한 적용이 각각 들어 있다. 지금은 같은 결과를 내지만 규칙을 바꿀 때 함께 수정해야 한다.
OverdueFeePolicy의 상수 선언은 숫자의 업무 의미와 단위를 드러낸다. 더 중요한 변화는 상수 자체보다 계산을 소유하는 위치다. 유예 일수를 빼고, 음수가 되지 않게 하고, 하루 요금을 곱하고, 상한을 적용하는 순서가 한 메서드에만 남는다.
feeForOverdueDays의 반환식에서 곱셈은 long 형식으로 수행된다. 금액 관련 상수에 L을 붙였기 때문이다. 클래스를 옮기는 과정에서 계산의 자료형까지 의도치 않게 바뀌지 않았는지 확인할 필요가 있다. 이번 수정은 계산 방식과 결과 형식을 유지한다.
LoanService의 생성자는 조회 동작과 계산 객체를 받는다. 서비스 안에서 실제 조회 객체를 생성하지 않으므로, 호출자가 다른 조회 구현을 전달할 수 있다. 요금 계산 객체는 현재 필요한 구체 클래스를 받는다. 두 의존에 서로 다른 선택을 한 이유가 코드에서 드러난다.
returnNotice가 quotedFeeWon을 호출하는 부분은 두 출력 경로가 같은 계산으로 모이는 지점이다. 안내 문구 메서드가 계산식을 다시 갖지 않게 했다. 계산 클래스만 추가하고 기존 계산식을 남겼다면 규칙 중복은 해결되지 않았을 것이다.
check와 failures는 확인 결과를 출력하고 실패 수를 센다. 모든 사례를 실행한 뒤 실패가 있으면 AssertionError를 던진다. 따라서 실패 문구를 출력했는데 프로그램이 정상 종료되어 문제가 가려지는 일을 줄인다. 성공할 때는 마지막까지 실행한 뒤 종료한다.
cases의 여섯 항목은 계산 전, 유예 끝, 과금 시작, 상한 직전, 상한 도달, 상한 초과를 고른 것이다. 가능한 모든 정수를 검사한 것은 아니다. 비교한 사례와 입력 전제를 함께 읽어야 하며, 이 결과만으로 모든 입력의 동작이 증명되었다고 표현해서는 안 된다.
반복문 안의 첫 check는 금액과 안내 문구를 모두 비교한다. 둘 중 하나라도 다르면 전후 동일 확인이 실패한다. 두 번째 check는 수정 후 금액을 직접 적은 기대값과 비교한다. 이렇게 동작 보존과 규칙 확인의 목적을 나누면 실패 원인을 해석하기 쉽다.
fixedLookup을 전달하는 부분은 저장 클래스를 만들지 않고도 서비스를 실행할 수 있음을 보여 준다. 항상 연체 3일인 대출을 돌려주는 조회 동작을 전달했다. 이 확인은 실제 저장소의 기능을 검증하는 것이 아니라, 서비스가 조회 인터페이스만으로 동작하는지를 확인한다.
실행 결과
Main.java가 있는 디렉터리에서 실행한다. out 디렉터리에는 컴파일 결과가 생성되며, 프로그램 자체는 파일을 쓰지 않는다. 추가로 컴파일러 경고까지 확인하려면 다음처럼 모든 경고를 활성화하고 경고를 오류로 처리할 수 있다.
javac -Xlint:all -Werror -d out *.java
java -cp out Main기본 명령인 javac -d out *.java로도 컴파일할 수 있다. 위 명령으로 컴파일할 때 별도 메시지가 나오지 않으며, 실행 출력은 다음과 같다.
PASS 전후 동일: 0일
PASS 규칙 기대값: 0일
PASS 전후 동일: 2일
PASS 규칙 기대값: 2일
PASS 전후 동일: 3일
PASS 규칙 기대값: 3일
PASS 전후 동일: 6일
PASS 규칙 기대값: 6일
PASS 전후 동일: 7일
PASS 규칙 기대값: 7일
PASS 전후 동일: 9일
PASS 규칙 기대값: 9일
PASS 조회 구현 교체
대출 L9: 연체료 500원
실패 수: 0전후 동일이라는 출력은 계산 결과뿐 아니라 안내 문구도 같다는 뜻이다. 예를 들어 금액을 그대로 유지하면서 문구의 공백을 바꾸면 해당 확인은 실패한다. 이번에는 출력 문구를 바꾸는 요청이 없으므로 문자열 전체를 비교한다. 문구 변경이 요구사항이라면 기대하는 새 문구를 따로 정하고 그 변경을 설명해야 한다.
실무에서 자주 틀리는 것
다음 코드는 각각 독립적으로 컴파일할 수 있는 작은 프로그램이다. 각 쌍은 같은 입력에 같은 값을 출력한다. 출력이 같다는 사실과 설계가 같다는 사실을 구별하기 위한 예다.
숫자만 모으고 규칙 중복은 남긴다
틀린 코드는 하루 요금과 상한을 상수로 모았지만 계산 순서는 두 메서드에 남아 있다. 유예 일수 적용 방식이 달라지면 여전히 두 곳을 수정해야 한다.
class DuplicateRule {
static final long DAILY_WON = 100L;
static final long MAX_WON = 500L;
static long quote(int overdueDays) {
return Math.min(MAX_WON, Math.max(0, overdueDays - 2) * DAILY_WON);
}
static long noticeFee(int overdueDays) {
return Math.min(MAX_WON, Math.max(0, overdueDays - 2) * DAILY_WON);
}
public static void main(String[] args) {
System.out.println(quote(3) + "/" + noticeFee(3));
}
}고친 코드는 두 경로가 같은 계산을 호출한다. 정책 변경 시 확인할 계산 위치가 하나가 된다. 두 프로그램 모두 100/100을 출력하지만, 다음 정책 변경에서 수정할 지점은 다르다.
class SharedRule {
static long fee(int overdueDays) {
return Math.min(500L, Math.max(0, overdueDays - 2) * 100L);
}
static long quote(int overdueDays) {
return fee(overdueDays);
}
static long noticeFee(int overdueDays) {
return fee(overdueDays);
}
public static void main(String[] args) {
System.out.println(quote(3) + "/" + noticeFee(3));
}
}생성자로 받으면 구체 클래스 의존도 사라졌다고 여긴다
틀린 코드는 객체 생성 위치를 옮겼지만, 서비스가 여전히 MemoryDays 형식만 받는다. 생성자로 받는 일과 필요한 동작에 의존하는 일은 구별해야 한다.
class ConcreteDependency {
static final class MemoryDays {
int findDays(String loanId) {
return 3;
}
}
static final class Service {
private final MemoryDays source;
Service(MemoryDays source) {
this.source = source;
}
int overdueDays(String loanId) {
return source.findDays(loanId);
}
}
public static void main(String[] args) {
System.out.println(new Service(new MemoryDays()).overdueDays("L3"));
}
}고친 코드는 조회 동작을 형식으로 받는다. 이 예제에서는 동작 하나뿐이므로 람다식으로 구현을 전달한다. 두 프로그램의 출력은 3으로 같다. 수정 후에는 서비스의 선언을 바꾸지 않고 다른 조회 동작을 전달할 수 있다.
class BehaviorDependency {
interface DayLookup {
int findDays(String loanId);
}
static final class Service {
private final DayLookup source;
Service(DayLookup source) {
this.source = source;
}
int overdueDays(String loanId) {
return source.findDays(loanId);
}
}
public static void main(String[] args) {
DayLookup fixed = loanId -> 3;
System.out.println(new Service(fixed).overdueDays("L3"));
}
}이름만 바꾸면서 정책도 함께 수정한다
틀린 코드는 이름을 명확하게 바꾼다고 하면서 유예 일수도 2일에서 3일로 바꾼다. 한 입력에서 결과가 같으면 이런 변경이 가려질 수 있다. 아래의 0일 입력에서는 두 값 모두 0이다.
class MixedChange {
static long calc(int days) {
return Math.min(500L, Math.max(0, days - 2) * 100L);
}
static long feeForOverdueDays(int overdueDays) {
return Math.min(500L, Math.max(0, overdueDays - 3) * 100L);
}
public static void main(String[] args) {
System.out.println(calc(0) + "/" + feeForOverdueDays(0));
System.out.println(calc(3) + "/" + feeForOverdueDays(3));
}
}첫 줄은 0/0이지만 둘째 줄은 100/0이다. 고친 코드는 유예 일수를 유지한다. 이름 변경과 정책 변경을 분리하면 리뷰어가 각 변경의 목적에 맞게 확인할 수 있다. 아래 출력은 차례대로 0/0과 100/100이다.
class NamingOnly {
static long calc(int days) {
return Math.min(500L, Math.max(0, days - 2) * 100L);
}
static long feeForOverdueDays(int overdueDays) {
return Math.min(500L, Math.max(0, overdueDays - 2) * 100L);
}
public static void main(String[] args) {
System.out.println(calc(0) + "/" + feeForOverdueDays(0));
System.out.println(calc(3) + "/" + feeForOverdueDays(3));
}
}이 예에서는 이름 변경의 영향을 보여 주려고 두 메서드를 함께 남겼다. 실제 반영에서는 호출부를 새 이름으로 바꾸고 필요 없는 옛 메서드를 제거한다. 동작 비교를 위한 임시 중복이 운영 코드의 새 중복으로 굳어지지 않게 정리 범위를 확인해야 한다.
한눈에 보기
| 냄새 | 리뷰에서 묻는 것 | 이번 대응 | 확인 근거 |
|---|---|---|---|
| 여러 책임의 혼합 | 서로 다른 요청이 같은 세부 코드를 바꾸는가 | 흐름과 요금 계산을 나눈다 | 계산식이 정책 클래스에 모인다 |
| 같은 규칙의 중복 | 정책 변경 시 함께 고칠 곳이 있는가 | 두 출력 경로가 같은 계산을 호출한다 | 금액과 안내 문구의 전후 비교 |
| 구체 클래스 의존 | 조회 구현 교체가 서비스 수정을 요구하는가 | 조회 인터페이스를 받는다 | 고정 조회 동작으로 서비스 실행 |
| 의도가 약한 이름 | 입력의 의미와 결과 단위를 읽을 수 있는가 | 연체 일수와 원 단위를 드러낸다 | 호출부에서 역할을 설명할 수 있다 |
| 과도한 변경 범위 | 동작 변경과 구조 변경이 섞였는가 | 문구 확장은 다음 변경 후보로 남긴다 | 현재 문구와 규칙을 유지한다 |
설계 냄새를 찾는 일은 분리할 클래스를 세는 일이 아니다. 같은 변경이 어디까지 퍼지는지 설명하고, 이번 변경에서 줄일 수 있는 부담을 고르는 일이다. 리뷰 의견에는 관찰한 위치, 예상되는 변경 위험, 제안한 범위, 확인 방법이 함께 드러나야 한다.
연습 문제
- 요금 계산을 OverdueFeePolicy로 옮겼지만 returnNotice에는 기존 계산식이 남아 있다고 하자. 현재 모든 확인이 통과할 수도 있는 이유와, 코드 구조에서 추가로 확인할 사항을 설명하라.
- 다른 도서관 서비스의 예약 취소 비용도 우연히 같은 숫자로 계산된다. 두 계산을 하나의 공통 메서드로 합치자는 의견에 답하라. 무엇을 먼저 확인해야 하는가.
- 완성 코드에 연체 4일인 대출 L4와 기대 금액 200원을 추가하라. 사례 배열과 조회용 맵 중 한쪽에만 추가하면 어떤 일이 생기는지 설명하라.
- 리뷰 중 안내 문구를 여러 언어로 제공하자는 제안이 나왔다. 현재 요청은 요금 규칙의 중복 제거다. 이번 변경의 범위와 다음 변경을 다시 검토할 조건을 담은 리뷰 의견을 작성하라.
정답과 해설
-
두 계산식이 현재 같은 규칙을 구현한다면 선택한 입력의 결과도 같아서 확인이 통과할 수 있다. 출력 비교만으로 중복 제거 여부를 판단할 수는 없다. returnNotice가 공통 계산 경로를 호출하는지 읽고, 업무 계산식이 남아 있지 않은지 확인해야 한다. 실행 확인은 동작 보존을 뒷받침하고, 코드 읽기는 변경 목적이 실제로 달성되었는지 확인한다.
-
두 비용이 같은 이유로 함께 바뀌어야 하는지 먼저 확인한다. 연체료와 예약 취소 비용의 정책 결정자가 다르거나 변경 조건이 다르면 별도 규칙일 수 있다. 현재 숫자가 같다는 이유로 합치면 한쪽 정책 변경이 다른 업무에 영향을 줄 수 있다. 계산 표현의 유사성보다 업무 규칙의 소유와 변경 이유를 근거로 판단한다.
-
사례 배열에는 L4, 4, 200L을 가진 FeeCase를 추가하고 맵에는 식별자 L4와 연체 일수 4인 Loan을 추가한다. 그러면 반복문에서 전후 동일 확인과 규칙 기대값 확인이 각각 하나씩 늘어난다. 배열에만 추가하면 조회할 대출이 없어 예외가 발생한다. 맵에만 추가하면 데이터는 있지만 반복문이 그 대출을 선택하지 않아 확인이 늘어나지 않는다.
-
예시 의견은 다음과 같다.
여러 언어의 문구를 지원하게 되면 문구 생성 책임을 별도로 나누는 것이 도움이 될 수 있다. 이번 변경은 요금 규칙을 한곳으로 모으고 기존 출력을 유지하는 범위로 두자. 두 번째 언어의 문구를 추가하는 변경에서 문구 생성 객체와 전달할 값을 함께 검토하자.
현재 범위를 정하면서도 후속 검토의 계기를 구체적으로 남겼다. 아직 요구되지 않은 구조를 먼저 늘리지 않고, 실제 변경이 생길 때 필요한 책임 분리를 판단할 수 있다.
READER FEEDBACK
질문·의견
내용에 관한 질문이나 더 나은 설명을 위한 의견을 남겨 주세요. 오탈자는 위의 제보 양식이 더 빨리 반영됩니다. 이 댓글은 원래 게시글과 같은 자리에 쌓입니다.
댓글 0
아직 댓글이 없습니다. 첫 댓글을 남겨 보세요.