이름과 다른 뜻으로 쓰고 있었다

리워드 지급 결과를 담는 참·거짓 변수가 있었다. 이름은 건너뛴다는 뜻인데 어떤 자리에서는 실패했다는 뜻으로 쓰고 있었다.

Open Table of contents

두 가지 뜻이 섞였다
이름을 믿는 코드가 생긴다
세 가지로 나눴다
이름만 고쳐도 되는 경우
정리

두 가지 뜻이 섞였다

Java 쪽에 boolean alreadyGiven 필드가 있었다. 이미 지급된 건이면 true로 둔다. 이후 처리를 건너뛰는 용도였다. 나중에 지급 실패에도 true를 넣는 코드가 붙었다. 둘 다 건너뛴다는 점이 같아서였다.

문제는 그 뒤에 붙은 코드였다. alreadyGiven이 true면 이미 지급됐다고 log4j에 남기고 있었다. 실패한 건도 이미 지급됨으로 기록되니 Tomcat 로그를 봐도 실패를 찾을 수 없다.

이름을 믿는 코드가 생긴다

alreadyGiven은 이미 지급됐다는 뜻이다. 다음 사람은 그렇게 읽고 코드를 붙인다. 집계 화면을 만들 때 JSP에서 alreadyGiven이 true인 건수를 셌다. 중복 방지 건수로 본 것이다.

그 숫자에 실패 건이 섞여 있었다. 화면은 3건이라는데 실제로는 중복 1건에 실패 2건이다. 이름을 믿은 코드가 틀렸고 그 코드에는 잘못이 없다.

세 가지로 나눴다

두 상황을 다른 값으로 나눴다. boolean은 두 가지만 담는다. 세 가지가 되는 순간 안 맞는다. Java 5의 enum을 썼다.

enum GiveResult {
    SUCCESS,      // 지급 완료
    ALREADY,      // 이미 지급됨
    FAILED        // 지급 실패
}

MySQL 집계 쿼리도 GiveResult 세 갈래로 나누니 실패 2건이 보인다. GROUP BY 한 줄이면 된다. 뭉쳐 있을 때는 안 보이던 것이다. 나중에 네 번째가 생기면 enum에 값을 하나 더 넣으면 된다. switch에서 빠뜨리면 Eclipse가 경고한다.

다른 데도 같은 문제가 있는지 boolean 필드를 훑어 보니 31개였고 그중 셋이 같은 모양이었다. hasError는 오류와 처리할 게 없음을 같이 담았다. isValid는 검증 실패와 값 없음을, isSent는 발송 성공과 대상 아님을 담고 있었다. 전부 이유가 다른데 하나로 뭉쳐 있었다.

이름만 고쳐도 되는 경우

전부 열거형으로 바꿀 필요는 없었다. 담는 뜻이 하나면 이름만 정확하게 고치면 된다.

건너뛴다는 이름을 중복이라서 건너뛴다는 이름으로 바꿨다. 이름에 조건이 들어가면 다른 용도로 쓰기 어려워진다. 그 이름에 실패를 담으려고 하면 이상하다는 것이 바로 보인다.

찾다 보니 부정형 이름도 있었다. 안 쓴다는 이름의 변수에 부정 연산자를 붙여 쓰고 있어서 읽을 때마다 두 번 뒤집어야 했다. 긍정으로 바꾸니 그냥 읽힌다.

enum으로 바꾸는 것이 항상 답은 아니었다. 참·거짓이 정말 둘뿐인 것도 있다.

isDeleted는 지워졌거나 아니거나 둘뿐이다. isAdmin도 그렇다. 이런 것은 boolean이 맞다.

가른 기준은 「나중에 세 번째가 생길 수 있나」였다. GiveResult는 나중에 PENDING이 생길 수 있고 isDeleted는 그렇지 않다.

이름만 고쳐도 되는 것도 있었다. flag나 check 같은 이름은 뜻이 없어서 isExpired처럼 상태를 담게 바꿨다. 값 구조는 그대로 두고 Eclipse의 Rename 리팩터링으로 이름만 바꿨다. JSP 안의 EL 표현식은 Eclipse가 안 잡아서 손으로 찾았다.

정리

이름이 뜻하는 것과 다르게 쓰면 그 이름을 믿은 코드가 틀린다
이유가 다른 상황을 하나의 참·거짓에 담으면 구분이 사라진다
세 가지 이상이면 참·거짓이 처음부터 안 맞으니 열거형으로 나눈다
나누면 집계에서도 각각이 보인다. 뭉쳐 있으면 숨는다
담는 뜻이 하나면 이름만 정확하게 고쳐도 된다
이름에 조건이 들어가면 다른 용도로 쓰려 할 때 이상하다는 게 보인다
부정형 이름에 부정 연산자를 붙이면 읽을 때마다 두 번 뒤집어야 한다