10,401줄을 지웠다 — 같은 화면이 두 벌씩 있던 관리자 정리하기

Level Up 개발자

목록 보기

27/30

관리자에 학생 상세 화면이 두 개 있었다. 경로가 다르고, 들어가는 메뉴가 다르고, 화면은 거의 같았다.

조직이 커지면서 두 팀이 각자 필요한 화면을 만들었고, 둘 다 "학생 한 명의 모든 것"을 보여줘야 했으니 자연스럽게 같은 걸 두 번 만들었다. 누구의 잘못도 아니다. 다만 이제는 탭 하나를 고치려면 두 군데를 고쳐야 하고, 한쪽만 고치면 두 화면이 서로 다른 말을 한다.

먼저 "같다"를 센다

합치기 전에 알아야 할 건 하나다. 얼마나 같은가.

눈으로 보면 "거의 같은데요"가 나온다. 그걸로는 합칠 수 없다. 거의 같다는 건 다른 데가 있다는 뜻이고, 그 다른 데가 의도된 것인지 사고인지 모르면 합치는 순간 기능이 사라진다.

그래서 두 화면의 구성 요소를 전부 나열하고 하나씩 대조했다.

	개수
양쪽에 있고 구현이 같음	50
양쪽에 있는데 의도적으로 다름	9
합계	59

50개가 10,401줄이었다. 이게 두 벌 있었다.

그리고 9개는 진짜로 달라야 하는 것들이었다 — 리포트 구성, 상담 이력 범위처럼 보는 사람의 권한과 목적이 달라서 다른 것들. 이걸 억지로 합치면 조건문이 화면마다 갈라지면서 결국 더 나빠진다.

합치지 않을 것을 먼저 정한다

리팩터링에서 제일 위험한 순간은 "이것도 비슷하니까 같이" 하는 순간이다. 그래서 순서를 뒤집었다. 9개를 먼저 격리하고, 나머지를 합쳤다.

공통 50개 + 경로별 어댑터 9개

공통 컴포넌트는 자기가 어느 화면에 그려지는지 모른다. 경로별 차이는 어댑터가 들고 있다가 주입한다. 그래서 두 화면의 차이가 "코드 여기저기에 흩어진 if"가 아니라 어댑터 파일 하나에 모인다.

리뷰할 때도 이게 낫다. 다음에 누가 "이 화면만 다르게 해주세요"라고 하면, 바뀌는 파일이 어댑터라는 게 diff 에 바로 보인다. 공통 파일이 바뀌면 양쪽에 영향 간다는 뜻이고, 그건 리뷰어가 반드시 알아야 하는 사실이다.

다시 흩어지지 않게 하는 게 진짜 작업이다

합치는 건 한 번 하면 끝이다. 문제는 6개월 뒤다.

누군가 공통 컴포넌트 안에서 if (pathname.startsWith('/crm')) 를 쓰는 순간, 공통은 공통이 아니게 된다. 겉보기엔 한 곳이지만 실제로는 다시 두 벌이고, 이제는 한 파일 안에서 두 벌이라 더 나쁘다.

그래서 구조를 검사하는 테스트를 남겼다.

// 공통 코드가 특정 라우트를 알면 안 된다
it('공통 컴포넌트는 라우트를 역참조하지 않는다', () => {
  const offenders = readFiles('components/student-detail/**')
    .filter((f) => /['"]\/(crm|teacher)\//.test(f.source));
  expect(offenders).toEqual([]);
});

// 옮기기로 한 50개가 실제로 거기 있는지
it('이동 대상 50개가 공통 디렉터리에 있다', () => {
  expect(missing(MOVED_COMPONENTS)).toEqual([]);
});

코드 리뷰로 잡겠다고 하면 안 잡힌다. 리뷰어는 바쁘고, 그 한 줄은 합리적으로 보인다. CI 가 막아야 안 돌아간다.

이게 리팩터링 PR 에서 가장 오래 남는 산출물이라고 생각한다. 지운 10,401줄보다 이 테스트 두 개가 더 값어치 있다.

검증 못 한 것도 적어야 한다

이 PR 에는 이렇게 적혀 있다.

인증 정보와 권한이 확인된 비민감 테스트 학생 계정이 이 작업 환경에 없어 실제 화면 픽셀 비교는 미실행입니다. 자동 검증과 구조 검사는 통과했습니다.

"화면은 그대로입니다"라고 쓰고 싶었다. 그런데 증명할 수가 없었다. 로그인이 필요한 화면이고, 테스트 계정 권한이 준비되지 않았다.

그럴 때 선택지는 세 개다.

"화면 변화 없음"이라고 쓴다 — 확인 안 했으면 거짓말이다
스크린샷 섹션을 지운다 — 리뷰어는 확인된 줄 안다
못 했다고 쓴다

3번을 골랐다. 리뷰어가 "그럼 내가 직접 띄워볼게요"라고 판단할 수 있어야 하고, 나중에 회귀가 나면 어디까지 검증됐는지가 기록에 남아 있어야 한다.

리팩터링 PR 에서 "화면은 그대로입니다"는 주장이지 사실이 아니다. 증명했으면 증명을 붙이고, 못 했으면 못 했다고 쓰는 게 맞다. (우리 저장소에는 이걸 픽셀로 증명하는 CLI 가 따로 있는데, 그건 로그인 없는 화면에서만 쓸 수 있었다.)

숫자로 남은 것

123 files changed   +1,124 / −10,549

지운 줄이 추가한 줄의 9배다. 이런 PR 이 좋은 PR 이라고 생각한다. 기능은 하나도 안 늘었고, 앞으로 고칠 곳이 절반이 됐다.

그런데 이 숫자가 목표였던 건 아니다. 목표는 "탭 하나 고칠 때 한 군데만 고치기" 였고, 줄 수는 그 결과일 뿐이다. 줄 수를 목표로 삼으면 9개의 의도된 차이까지 합쳐버리게 된다.

같은 관리자 정리 작업에서 나온 다음 이야기는 조금 다른 종류다. 요청받은 대시보드 위젯 5개 중 2개만 만들고, 나머지 3개는 왜 만들 수 없는지를 표로 적어 낸 일.

하승진

기어갈지언정 한 발자국씩이라도 가보자

이전 포스트

같은 이름의 함수 두 개가 실패할 때 서로 다르게 동작했다

다음 포스트