결산 검증기(
SettlementValidator)를 만들던 중 IDE가Optional매개변수에 경고를 띄운 것을 확인했다. 우리 코드의 경우 작동 관점에서 실제로 문제가 되는 상황은 없었지만, 경고를 조사하며Optional매개변수가 왜 권장되지 않는지 정리하게 되었다. 고치는 과정에서 이와는 별개인 구조 문제도 하나 함께 발견해 정리했다. 어떤 코드였는지, 왜 경고가 뜨는지, 어떻게 고쳤는지를 짧게 남긴다.
Optional을 매개변수로 받는 검증 메서드결산 로그를 검증하는 validate가 열린 판을 Optional<OpenRun>으로 받고 있었다. 결산 검증의 첫 검사(S1)가 "열린 판이 있고 runId가 같은가"라서, 열린 판이 없을 수 있다는 사실을 그대로 타입에 담은 것이다.
public Optional<Violation> validate(SettlementPayload payload, RunSetup setup,
Optional<OpenRun> openRun, long currentGold) {
Optional<Violation> found = checkRunOpen(payload, openRun); // S1
if (found.isPresent()) {
return found;
}
found = checkKills(payload, setup); // S2 ~
...
}
열린 판이 없을 수 있다는 사실을 타입으로 표현하려다 보니 Optional을 매개변수로 받게 된 것이다. 동작은 했고 테스트도 통과했다.
그런데 IDE가 "Optional<OpenRun>이(가) 매개변수 openRun에 사용됩니다."라는 경고를 표시했다. javac가 내는 경고가 아니라 IntelliJ 같은 IDE 검사나 SonarLint, ErrorProne 같은 정적 분석 도구가 내는 것이다.
Optional은 "결과가 없을 수 있다"를 표현하려고 만든 반환 타입 전용이다. JDK 설계자도 라이브러리 메서드의 반환값에서 "결과 없음"을 나타낼 명확한 필요가 있을 때를 위한 제한적인 장치로 만들었다고 밝혔다. 이 범위를 벗어난 매개변수나 필드에 쓰면 이점은 없고 문제만 생긴다.
Optional이 호출자가 없는 경우를 처리하게 강제한다. 매개변수에서는 호출자가 이미 값이 있는지 알고 있고, 강제할 상대가 따로 없다.Optional.of(...)로 감싸야 하고, 없으면 Optional.empty()를 만들어 넘겨야 한다.null을 없애려고 만든 타입인데, 매개변수에서는 null, Optional.empty(), 값 있음의 세 상태가 된다. Optional 자체가 null로 들어올 수도 있기 때문이다.Optional을 필드에 쓰는 것도 같은 이유로 안티패턴이고, IDE는 "Optional을 필드 타입으로 사용했다"는 검사도 따로 둔다.
Serializable이 아니다. Optional 필드를 가진 클래스는 직렬화할 수 없고, 필드가 Optional이라서 생기는 문제가 JSON 변환이나 JPA 같은 프레임워크 경계에서 드러난다.Optional 객체가 하나 더 생기고 한 번 더 거쳐 읽어야 한다. 필드는 객체의 상태를 그대로 담는 곳이라 "결과 없음"을 표현할 이유가 없다.null 문제가 없어지지 않는다. 필드 자체가 null일 수 있고, 생성자나 세터에서 Optional이 아니라 null이 들어오는 경우를 따로 막아야 한다.값이 없을 수 있는 필드는 평범한 타입에 null을 허용하고, 그 값을 꺼내는 getter의 반환 타입에서만 Optional로 감싸 돌려주는 방식이 일반적이다.
같은 Optional이라도 파라미터나 필드와는 달리 반환값은 null을 따로 고려하지 않아도 된다. Optional을 반환하는 메서드가 null을 돌려주는 것은 구현의 버그이고, 부재는 Optional.empty() 하나로만 표현한다는 것이 이 타입의 규약이기 때문이다.
가장 흔한 방법은 Optional 대신 실제 타입을 그대로 받고, 메서드 안에서 null을 검사하는 것이다.
public Optional<Violation> validate(SettlementPayload payload, RunSetup setup,
OpenRun openRun, long currentGold) {
if (openRun == null || !openRun.runId().equals(payload.runId())) {
return violation(RejectCode.RUN_NOT_OPEN, "...");
}
...
}
Optional이 하나 줄고 호출자의 감싸는 코드도 없어진다. 대신 "null이 올 수 있다"는 사실을 문서나 어노테이션(@Nullable 등)으로 따로 알려야 한다. 그 밖에 오버로드로 "있는 경우"와 "없는 경우"를 나누는 방법도 있다.
고치다 보니 우리 코드에는 Optional 매개변수와는 별개로 구조적인 불안함이 있었다.
Optional 매개변수위 3장처럼 OpenRun을 받고 null을 검사하는 것으로 경고는 없앨 수 있었다. 하지만 먼저 이 메서드가 열린 판을 받아야 하는지부터 따져 봤다. 열린 판은 S1에서만 쓰였고, 호출하는 쪽(SyncContext)이 이미 열린 판을 알고 있었다. 그래서 S1은 SyncContext를 받아 안에서 openRun()을 읽고, 매개변수에서 Optional을 없앴다. Optional<OpenRun>은 context.openRun()의 반환값으로만 남았고, 여기서는 의도에 맞는 사용이다.
null 설정처리기는 판 설정(RunSetup)을 열린 판의 시작 성장도로 만든다. 열린 판이 없으면 설정을 만들 수 없으니, 처리기는 이렇게 우회하고 있었다.
if (openRun.isEmpty() || !openRun.get().runId().equals(payload.runId())) {
// 설정을 만들 수 없으니 null을 넘기고, 검증기가 S1에서 먼저 거부해 주기를 기대한다
return rejected(validator.validate(payload, null, openRun, context.gold()));
}
setup에 null을 넘기고 "검증기가 첫 검사에서 끝내니까 안 쓰일 것"이라고 믿는 코드다. 이것은 Optional 때문에 생긴 문제가 아니다. openRun을 OpenRun으로 받아 null 검사를 해도 똑같이 남는다. "열린 판이 있는가"라는 검사와 "열린 판 기준으로 값을 검증한다"는 단계를 한 메서드에 묶어서 생긴 문제다.
해결은 S1과 나머지를 나누는 것이었다.
// S1: 열린 판이 있고 runId가 같다
public Optional<Violation> checkRunOpen(SettlementPayload payload, SyncContext context) { ... }
// S2 ~ S9: S1을 통과한 결산만 받는다
public Optional<Violation> validate(SettlementPayload payload, RunSetup setup, long currentGold) { ... }
처리기는 순서대로 호출하고, 열린 판이 확인된 뒤에야 설정을 만든다.
Optional<Violation> violation = validator.checkRunOpen(payload, context);
if (violation.isPresent()) {
return rejected(violation);
}
OpenRun openRun = context.openRun().orElseThrow();
RunSetup setup = rules.setupFor(openRun.startGrowthStage(), rules.upgradesFor(context.nodeRanks()));
violation = validator.validate(payload, setup, context.gold());
"검증기가 알아서 먼저 끝내 줄 것"이라는 암묵적인 약속 대신 호출 순서가 그 역할을 한다. validate는 열린 판이 있는 결산만 받으므로 설정이 null일 일도 없다.
한 번의 리팩터링으로 두 문제가 함께 풀렸지만, 원인은 다르다. 문제 1은 3장의 일반적인 개선안으로도 풀리고, 문제 2는 그것으로 풀리지 않는다.
검사 순서와 거부 코드는 그대로여서 테스트 31개는 수정 없이 통과했다.
Optional은 반환 타입에만 쓴다. 매개변수와 필드에서는 이점은 없고 호출자의 부담, null의 경우의 수, 직렬화 문제만 늘어난다.Optional을 실제 타입과 null 검사로 바꾸거나, 오버로드로 나누거나, 애초에 그 값을 받을 필요가 있는지 점검하는 것이다.null을 넘기던 구조였고, Optional과는 별개로 단계를 분리해서 풀었다.