리팩토링 실전

이론은 충분해, 이제 실전이야. 자기 코드를 점진적으로 개선하고, 남이 잘 만든 코드도 더 깨끗하게 다듬고, 낯선 코드를 돌려보고 고쳐보는 — 리팩터링의 세 가지 풍경을 구경하는 챕터야.

저자가 커맨드라인 인수를 파싱하는 Args 유틸리티를 처음부터 끝까지 만들어. 형식 문자열 "l,p#,d*"에서 l은 boolean, p#은 정수, d*은 문자열을 의미하고, 커맨드라인에서 -l -p 8080 -d /usr/logs처럼 넘기면 알아서 파싱해주는 거야. 최종 완성된 코드는 깨끗해. ArgumentMarshaler라는 추상 클래스를 기반으로 BooleanArgumentMarshaler, StringArgumentMarshaler, IntegerArgumentMarshaler 같은 파생 클래스가 각 타입을 처리하고, 각 클래스가 하나의 책임만 가지고, 새 인수 타입을 추가하려면 새 Marshaler 클래스만 만들면 돼.

근데 저자가 강조하는 건 — 이 깨끗한 코드가 처음부터 이랬던 게 아니라는 거야. 1차 초안은 돌아가기는 했어. Boolean과 String 인수만 지원하는 상태에서는 그럭저럭 봐줄 만했지. 그런데 여기에 Integer 인수 타입을 추가하면서 코드가 "급격히 나빠지기 시작"했어. 타입 하나 추가할 때마다 parse, get, set 메서드를 고치고, 오류 처리를 추가하고, 새로운 Map을 선언해야 했거든. 기존의 if-else 체인이 점점 길어졌지.

저자는 이 시점에서 멈춰. double 인수 타입까지 추가하면 코드가 완전히 통제 불능이 될 거라는 걸 알았으니까. 이게 핵심 교훈이야 — 코드가 나빠지고 있다는 걸 알면서도 "일단 기능부터 넣자"고 밀어붙이면 안 돼. "멈추고 정리해야 하는 시점"을 인식하는 게 프로의 역량이야.

저자는 TDD를 활용해서 점진적으로 구조를 개선해. 테스트 주도로 안전망을 깔아 — 리팩터링 전에 테스트 슈트를 완성하고, 모든 테스트가 통과하는 상태에서 시작해서, 한 번에 하나씩 바꾸고, 바꿀 때마다 테스트를 돌려. 테스트가 깨지면 즉시 되돌리지. 한 번에 하나씩 — 전체 구조를 한 번에 바꾸지 않아. Boolean 인수부터 ArgumentMarshaler 패턴으로 변환하고, 그게 완전히 끝나면 String을 변환하고, 그 다음 Integer를 변환해. 매 단계마다 테스트를 돌려. "프로그램을 망치는 가장 좋은 방법은 개선이라는 이름 아래 구조를 크게 뒤집는 것"이야. 한 번에 한 가지만 바꾸면 구조를 크게 개선하면서도 프로그램이 항상 돌아가는 상태를 유지할 수 있거든.

자기 코드를 개선했다면, 이제 남의 코드를 개선하는 연습도 필요해. JUnit은 자바 진영에서 가장 유명한 테스트 프레임워크고, 코드 품질이 높은 것으로 유명한데, 그런데도 개선할 여지가 있다는 게 핵심이야. 저자가 분석 대상으로 고른 건 ComparisonCompactor라는 클래스야. assertEquals(expected, actual)에서 두 문자열이 다를 때, 어디가 다른지 한눈에 보여주는 역할을 해. 원래 코드도 상당히 깨끗해. 테스트 커버리지가 100%이고, 켄트 벡이 원래 만든 코드니까.

그래도 보이스카우트 규칙에 따라, 저자는 더 깨끗하게 만들 수 있는 부분을 찾아. fExpected, fActual, fPrefix 같은 변수에서 f 접두어를 제거해. if (expected == null || actual == null || areStringsEqual()) 같은 조건문은 if (shouldNotCompact())로 캡슐화해. compact() 함수가 압축도 하고 형식도 맞추고 있었어. 이름과 다른 일을 하니까 형식을 맞추는 부분을 formatCompactedComparison()으로 뽑고, 실제 압축은 compactExpectedAndActual()이라는 별도 함수로 분리해. 그리고 findCommonSuffix()가 findCommonPrefix()에 의존하는 "시간적 결합(temporal coupling)"이 있었는데, findCommonSuffix(prefixIndex)처럼 인수로 의존성을 명시적으로 만들어.

마지막으로, 데이비드 길버트가 만든 JCommon 라이브러리의 SerialDate 클래스를 돌려보고 고쳐보는 과정이야. 먼저 테스트를 돌려봐. 테스트 커버리지가 낮다는 걸 발견하거든. 실행 가능한 문장 중 약 50%만 테스트하고 있었어. 저자는 독자적으로 테스트 케이스를 추가하고, 경계 조건을 테스트하고, 버그를 몇 개 발견하지.

테스트 안전망을 확보한 후 리팩터링을 시작해. 변경 이력 주석을 삭제 — Git이 관리하니까 소음일 뿐이야. SerialDate라는 이름 자체가 문제야. "Serial"이 직렬화(Serializable)를 연상시키는데, 실제로는 날짜의 일련번호를 뜻하거든. 저자는 DayDate로 이름을 바꾸자고 제안해. MonthConstants 인터페이스를 Month라는 enum으로 바꾸는 것도 큰 개선이야. Month.JANUARY가 JANUARY = 1 상수보다 훨씬 타입 안전하고 명확하거든. SerialDate가 추상 클래스인데, 내부에 구체적인 구현 세부사항이 들어 있었어. 추상 클래스는 추상적인 개념만 담아야 하고, 구현 세부사항은 파생 클래스에 있어야 해.

리팩터링 결과, 코드가 확 줄어들고 구조가 명확해졌어. 테스트 커버리지도 올라갔고, 버그도 잡았지. 나쁜 코드도 깨끗한 코드로 개선할 수 있어. 하지만 비용이 엄청나게 들어. 처음부터 깨끗하게 짜는 게 훨씬 싸.

---

정리

6장 읽고 기억할 거 세 가지:

1. 깨끗한 코드는 처음부터 깨끗했던 게 아니다. 1차 초안을 쓰고, 코드가 나빠지고 있다면 멈추고, 테스트를 만들고, 한 번에 하나씩 점진적으로 개선하는 과정을 거쳐서 나온 결과물이다
2. 좋은 코드도 더 좋아질 수 있다. 숨겨진 시간적 결합을 명시적으로 드러내고, 조건문을 캡슐화하고, 이름을 명확하게 바꾸라. 테스트 커버리지가 리팩터링의 안전망이다
3. 먼저 돌려보고, 그 다음 고쳐라. 테스트를 작성해서 현재 동작을 이해하고, 안전망을 확보한 후에 리팩터링하라. 이름이 중요하고, 추상 클래스에 구현 세부사항을 넣지 마라